From f38329189fddd5aa62d6e753474281fa77e57074 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Fri, 22 May 2026 14:13:32 -0700 Subject: [PATCH 1/5] Partial backport of platform PR #7435 (#7686) #### Rationale Simplify the `study` schema's queries. #### Changes - Remove unused table --- .../IVisualizationSourceQuery.java | 2 - .../labkey/study/query/StudyQuerySchema.java | 22 ---- .../query/VisualizationVisitTagTable.java | 104 ------------------ .../StudyVisualizationProvider.java | 41 ++++--- .../sql/OuterJoinSourceQuery.java | 6 - .../sql/VisualizationSourceQuery.java | 17 +-- 6 files changed, 24 insertions(+), 168 deletions(-) diff --git a/api/src/org/labkey/api/visualization/IVisualizationSourceQuery.java b/api/src/org/labkey/api/visualization/IVisualizationSourceQuery.java index 0fc293bb331..9c017057af5 100644 --- a/api/src/org/labkey/api/visualization/IVisualizationSourceQuery.java +++ b/api/src/org/labkey/api/visualization/IVisualizationSourceQuery.java @@ -63,8 +63,6 @@ public interface IVisualizationSourceQuery boolean isSkipVisitJoin(); - boolean isVisitTagQuery(); - /** * True if any select or aggregate requires a left join explicitly. This is an override for any columns * that might require some form of an INNER JOIN. diff --git a/study/src/org/labkey/study/query/StudyQuerySchema.java b/study/src/org/labkey/study/query/StudyQuerySchema.java index a5fbb61149e..83a2fb67591 100644 --- a/study/src/org/labkey/study/query/StudyQuerySchema.java +++ b/study/src/org/labkey/study/query/StudyQuerySchema.java @@ -127,7 +127,6 @@ public class StudyQuerySchema extends UserSchema implements UserSchema.HasContex public static final String VISIT_TAG_TABLE_NAME = "VisitTag"; public static final String VISIT_TAG_MAP_TABLE_NAME = "VisitTagMap"; public static final String VISIT_ALIASES = "VisitAliases"; - public static final String VISUALIZATION_VISIT_TAG_TABLE_NAME = "VisualizationVisitTag"; public static final String VISIT_MAP_TABLE_NAME = "VisitMap"; public static final String STUDY_DATA_TABLE_NAME = "StudyData"; @@ -645,27 +644,6 @@ public TableInfo createTable(String name, ContainerFilter cf) { return new VisitMapTable(this, cf); } - if (name.startsWith(VISUALIZATION_VISIT_TAG_TABLE_NAME)) - { - // Name is encoded with useProtocolDay boolean, tagName, and altQueryName - String params = name.substring(VISUALIZATION_VISIT_TAG_TABLE_NAME.length()); - boolean useProtocolDay; - if (params.startsWith("-true")) - { - params = params.substring(params.indexOf("-true") + 6); - useProtocolDay = true; - } - else - { - params = params.substring(params.indexOf("-false") + 7); - useProtocolDay = false; - } - int hyphenIndex = params.indexOf("-"); - String tagName = hyphenIndex > -1 ? params.substring(0, hyphenIndex) : params; - String altQueryName = hyphenIndex > -1 ? params.substring(hyphenIndex + 1) : null; - - return new VisualizationVisitTagTable(this, cf, getStudy(), getUser(), tagName, useProtocolDay, altQueryName); - } // Might be a dataset DatasetDefinition dsd = getDatasetDefinitionByQueryName(name); diff --git a/study/src/org/labkey/study/query/VisualizationVisitTagTable.java b/study/src/org/labkey/study/query/VisualizationVisitTagTable.java index b494591ef6e..e69de29bb2d 100644 --- a/study/src/org/labkey/study/query/VisualizationVisitTagTable.java +++ b/study/src/org/labkey/study/query/VisualizationVisitTagTable.java @@ -1,104 +0,0 @@ -/* - * Copyright (c) 2014-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.labkey.study.query; - -import org.jetbrains.annotations.NotNull; -import org.labkey.api.data.ContainerFilter; -import org.labkey.api.data.JdbcType; -import org.labkey.api.data.SQLFragment; -import org.labkey.api.data.VirtualTable; -import org.labkey.api.query.ExprColumn; -import org.labkey.api.security.User; -import org.labkey.api.study.StudyService; -import org.labkey.study.model.StudyImpl; - -public class VisualizationVisitTagTable extends VirtualTable -{ - private final StudyImpl _study; - private final User _user; - private final boolean _useProtocolDay; - private final String _visitTagName; - private final String _dayString; - private final String _altQueryName; - - public VisualizationVisitTagTable(StudyQuerySchema schema, ContainerFilter cf, StudyImpl study, User user, String visitTagName, boolean useProtocolDay, String altQueryName) - { - super(schema.getDbSchema(), "VizVisitTag", schema, cf); - _study = study; - _user = user; - _visitTagName = visitTagName; - _useProtocolDay = useProtocolDay; - if (_useProtocolDay) - _dayString = "ProtocolDay"; - else - _dayString = "Day"; - _altQueryName = altQueryName; - - addColumn(new ExprColumn(this, StudyService.get().getSubjectColumnName(_study.getContainer()), - new SQLFragment(ExprColumn.STR_TABLE_ALIAS + "." + "ParticipantId"), JdbcType.VARCHAR)); - - // 20546: need to expose Container for use in StudyVisualizationProvider.getJoinColumns - addColumn(new ExprColumn(this, "Container", new SQLFragment(ExprColumn.STR_TABLE_ALIAS + "." + "Container"), JdbcType.VARCHAR)); - - addColumn(new ExprColumn(this, "ZeroDay", new SQLFragment(ExprColumn.STR_TABLE_ALIAS + "." + _dayString), JdbcType.INTEGER)); - } - - @NotNull - @Override - public SQLFragment getFromSQL(String alias) - { - checkReadBeforeExecute(); - if (_altQueryName == null) - { - String innerAlias = alias + "_SP"; - String joinString; - if (_useProtocolDay) - { - joinString = "\nJOIN study.Visit ON " + innerAlias + ".VisitId = study.Visit.RowId AND " + - innerAlias + ".VisitId IS NOT NULL"; - } - else - { - joinString = "\nJOIN study.ParticipantVisit ON " + innerAlias + ".VisitId = study.ParticipantVisit.VisitRowId AND " + - innerAlias + ".ParticipantId = study.ParticipantVisit.ParticipantId"; - } - - SQLFragment from = new SQLFragment("(SELECT VisitId, " + _dayString + ", " + innerAlias + ".ParticipantId, Container " + " FROM (SELECT ParticipantId, "); - from.append("COALESCE(CohortVisitTag.VisitId, NoCohortTag.VisitId) As VisitId, CurrentCohortId FROM study.Participant\n") - .append("LEFT OUTER JOIN study.VisitTagMap CohortVisitTag ON study.Participant.CurrentCohortId = CohortVisitTag.CohortID AND CohortVisitTag.VisitTag=") - .append("'" + _visitTagName + "'\n") - .append("AND CohortVisitTag.Container = Participant.Container"); - from.append("\nLEFT OUTER JOIN study.VisitTagMap NoCohortTag ON NoCohortTag.VisitTag =") - .append("'" + _visitTagName + "' AND NoCohortTag.CohortID IS NULL\n") - .append("AND NoCohortTag.Container = Participant.Container"); - from.append("\nWHERE "); - - // TODO: this is a temp fix for the Dataspace usecase - from.append(ContainerFilter.Type.AllInProject.create(_study.getContainer(), _user).getSQLFragment(getSchema(), new SQLFragment("study.Participant.Container"))); - - from.append(") ").appendIdentifier(innerAlias); - from.append(joinString); - from.append("\n) ").appendIdentifier(alias); - return from; - } - else - { - // allow caller to pass in their own query to use for the SQL to get the participant-to-zero day map (used for CDS study axis alignment by visit tag) - // NOTE: it is assumed that the query will have the expected columns plus a column for VisitTagMap to filter on - return new SQLFragment("(SELECT * FROM " + _altQueryName + " WHERE VisitTagName = '" + _visitTagName + "')").appendIdentifier(alias); - } - } -} diff --git a/study/src/org/labkey/study/visualization/StudyVisualizationProvider.java b/study/src/org/labkey/study/visualization/StudyVisualizationProvider.java index 445a8eb9fff..353feb1e7dc 100644 --- a/study/src/org/labkey/study/visualization/StudyVisualizationProvider.java +++ b/study/src/org/labkey/study/visualization/StudyVisualizationProvider.java @@ -177,31 +177,28 @@ public List> getJoinC joinCols.add(new Pair<>(firstSubjectCol, secondSubjectCol)); - if (!first.isVisitTagQuery() && ! second.isVisitTagQuery()) - { - // attempt to lookup the dataset using the queryName by label and then by name - Dataset firstDataset = StudyService.get().resolveDataset(first.getContainer(), first.getQueryName()); - Dataset secondDataset = StudyService.get().resolveDataset(second.getContainer(), second.getQueryName()); + // attempt to lookup the dataset using the queryName by label and then by name + Dataset firstDataset = StudyService.get().resolveDataset(first.getContainer(), first.getQueryName()); + Dataset secondDataset = StudyService.get().resolveDataset(second.getContainer(), second.getQueryName()); - boolean subjectJoinOnly = isGroupByQuery || first.isSkipVisitJoin() || second.isSkipVisitJoin(); + boolean subjectJoinOnly = isGroupByQuery || first.isSkipVisitJoin() || second.isSkipVisitJoin(); - // if either query is a demographic dataset, it's sufficient to join on subject only: - if (!subjectJoinOnly && (firstDataset == null || firstDataset.getKeyType() != Dataset.KeyType.SUBJECT) && - (secondDataset == null || secondDataset.getKeyType() != Dataset.KeyType.SUBJECT)) + // if either query is a demographic dataset, it's sufficient to join on subject only: + if (!subjectJoinOnly && (firstDataset == null || firstDataset.getKeyType() != Dataset.KeyType.SUBJECT) && + (secondDataset == null || secondDataset.getKeyType() != Dataset.KeyType.SUBJECT)) + { + VisualizationSourceColumn firstSequenceCol = getVisitJoinColumn(factory, first, firstSubjectNounSingular); + VisualizationSourceColumn secondSequenceCol = getVisitJoinColumn(factory, second, secondSubjectNounSingular); + joinCols.add(new Pair<>(firstSequenceCol, secondSequenceCol)); + + // for datasets with matching 3rd keys, join on subject/visit/key (if neither are pivoted), allowing null results for this column so as to follow the lead of the primary measure column for this query: + if (firstDataset != null && firstDataset.getKeyType() == Dataset.KeyType.SUBJECT_VISIT_OTHER && + secondDataset != null && secondDataset.getKeyType() == Dataset.KeyType.SUBJECT_VISIT_OTHER && + first.getPivot() == null && second.getPivot() == null && firstDataset.hasMatchingExtraKey(secondDataset)) { - VisualizationSourceColumn firstSequenceCol = getVisitJoinColumn(factory, first, firstSubjectNounSingular); - VisualizationSourceColumn secondSequenceCol = getVisitJoinColumn(factory, second, secondSubjectNounSingular); - joinCols.add(new Pair<>(firstSequenceCol, secondSequenceCol)); - - // for datasets with matching 3rd keys, join on subject/visit/key (if neither are pivoted), allowing null results for this column so as to follow the lead of the primary measure column for this query: - if (firstDataset != null && firstDataset.getKeyType() == Dataset.KeyType.SUBJECT_VISIT_OTHER && - secondDataset != null && secondDataset.getKeyType() == Dataset.KeyType.SUBJECT_VISIT_OTHER && - first.getPivot() == null && second.getPivot() == null && firstDataset.hasMatchingExtraKey(secondDataset)) - { - VisualizationSourceColumn firstKeyCol = factory.create(first.getSchema(), first.getQueryName(), firstDataset.getKeyPropertyName(), true); - VisualizationSourceColumn secondKeyCol = factory.create(second.getSchema(), second.getQueryName(), secondDataset.getKeyPropertyName(), true); - joinCols.add(new Pair<>(firstKeyCol, secondKeyCol)); - } + VisualizationSourceColumn firstKeyCol = factory.create(first.getSchema(), first.getQueryName(), firstDataset.getKeyPropertyName(), true); + VisualizationSourceColumn secondKeyCol = factory.create(second.getSchema(), second.getQueryName(), secondDataset.getKeyPropertyName(), true); + joinCols.add(new Pair<>(firstKeyCol, secondKeyCol)); } } diff --git a/visualization/src/org/labkey/visualization/sql/OuterJoinSourceQuery.java b/visualization/src/org/labkey/visualization/sql/OuterJoinSourceQuery.java index 5b4b214e07c..e955a3d1554 100644 --- a/visualization/src/org/labkey/visualization/sql/OuterJoinSourceQuery.java +++ b/visualization/src/org/labkey/visualization/sql/OuterJoinSourceQuery.java @@ -235,12 +235,6 @@ public boolean isSkipVisitJoin() return false; } - @Override - public boolean isVisitTagQuery() - { - return false; - } - @Override public boolean isRequireLeftJoin() { diff --git a/visualization/src/org/labkey/visualization/sql/VisualizationSourceQuery.java b/visualization/src/org/labkey/visualization/sql/VisualizationSourceQuery.java index ba0b0901c54..9827a6e2410 100644 --- a/visualization/src/org/labkey/visualization/sql/VisualizationSourceQuery.java +++ b/visualization/src/org/labkey/visualization/sql/VisualizationSourceQuery.java @@ -131,12 +131,6 @@ public boolean requireInnerJoin() return false; } - @Override - public boolean isVisitTagQuery() - { - return _queryName.startsWith("VisualizationVisitTag"); - } - @Override public boolean isRequireLeftJoin() { @@ -213,7 +207,7 @@ public String getSelectListName(Set selectAliases) private static void addToColMap(Map> colMap, String name, VisualizationSourceColumn alias) { - Set aliases = colMap.computeIfAbsent(name, k -> new LinkedHashSet<>()); + Set aliases = colMap.computeIfAbsent(name, n -> new LinkedHashSet<>()); aliases.add(alias); } @@ -434,7 +428,7 @@ public String getGroupByClause() return ""; } - private void appendValueList(StringBuilder sql, VisualizationSourceColumn col) throws org.labkey.api.visualization.SQLGenerationException + private void appendValueList(StringBuilder sql, VisualizationSourceColumn col) { if (col.getValues() != null && !col.getValues().isEmpty()) { @@ -457,7 +451,7 @@ private void appendValueList(StringBuilder sql, VisualizationSourceColumn col) t } } - public String getPivotClause() throws org.labkey.api.visualization.SQLGenerationException + public String getPivotClause() { if (_pivot != null) { @@ -560,7 +554,7 @@ private String appendSimpleFilter(StringBuilder where, SimpleFilter filter, Stri return separator; } - public String getWhereClause() throws org.labkey.api.visualization.SQLGenerationException + public String getWhereClause() { StringBuilder where = new StringBuilder(); String sep = "WHERE "; @@ -598,12 +592,11 @@ public String getWhereClause() throws org.labkey.api.visualization.SQLGeneration @Override public String getSQL(VisualizationSourceColumn.Factory factory) throws SQLGenerationException { - String sql = getSelectClause(factory) + "\n" + + return getSelectClause(factory) + "\n" + getFromClause() + "\n" + getWhereClause() + "\n" + getGroupByClause() + "\n" + getPivotClause() + "\n"; - return sql; } @Override From f50a7f68771276de79be7f8d53a218e9f211ef68 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 28 May 2026 11:48:31 -0700 Subject: [PATCH 2/5] Safe redirect action (#7695) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### Rationale Simple server-side action to help with https://github.com/LabKey/internal-issues/issues/1023 #### Tasks 📍 - [x] Claude Code Review - [x] Code Review - [x] Manual Testing (server action) @cnathe - [x] Manual Testing (client) @labkey-adam Testing and verification of the issue fix will come after the client changes are implemented. #### Related PR - https://github.com/LabKey/testAutomation/pull/3023 --------- Co-authored-by: cnathe --- .../client/AssayDesigner/AssayDesigner.tsx | 5 ++-- .../DataClassDesigner/DataClassDesigner.tsx | 5 ++-- .../DatasetDesigner/DatasetDesigner.tsx | 5 ++-- .../client/DomainDesigner/DomainDesigner.tsx | 5 ++-- .../IssuesListDesigner/IssuesListDesigner.tsx | 5 ++-- core/src/client/ListDesigner/ListDesigner.tsx | 4 +-- .../SampleTypeDesigner/SampleTypeDesigner.tsx | 5 ++-- core/src/org/labkey/core/CoreController.java | 22 +++++++++++++--- .../labkey/test/tests/study/AssayTest.java | 26 ++++++++++++++++++- .../test/tests/study/StudyDatasetsTest.java | 19 ++++++++++++++ 10 files changed, 77 insertions(+), 24 deletions(-) diff --git a/core/src/client/AssayDesigner/AssayDesigner.tsx b/core/src/client/AssayDesigner/AssayDesigner.tsx index 518b97ca595..3634428ec7a 100644 --- a/core/src/client/AssayDesigner/AssayDesigner.tsx +++ b/core/src/client/AssayDesigner/AssayDesigner.tsx @@ -140,8 +140,9 @@ export class App extends React.Component { 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 => { diff --git a/core/src/client/DataClassDesigner/DataClassDesigner.tsx b/core/src/client/DataClassDesigner/DataClassDesigner.tsx index 4a19ca02075..422d3aac188 100644 --- a/core/src/client/DataClassDesigner/DataClassDesigner.tsx +++ b/core/src/client/DataClassDesigner/DataClassDesigner.tsx @@ -68,9 +68,8 @@ class DataClassDesignerWrapper extends React.Component { 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 = () => { diff --git a/core/src/client/DatasetDesigner/DatasetDesigner.tsx b/core/src/client/DatasetDesigner/DatasetDesigner.tsx index 50eec7df716..f30c22ebd52 100644 --- a/core/src/client/DatasetDesigner/DatasetDesigner.tsx +++ b/core/src/client/DatasetDesigner/DatasetDesigner.tsx @@ -66,9 +66,8 @@ class DatasetDesigner extends PureComponent { 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 { diff --git a/core/src/client/DomainDesigner/DomainDesigner.tsx b/core/src/client/DomainDesigner/DomainDesigner.tsx index 08ae19c2d91..240bf106d04 100644 --- a/core/src/client/DomainDesigner/DomainDesigner.tsx +++ b/core/src/client/DomainDesigner/DomainDesigner.tsx @@ -161,9 +161,8 @@ class DomainDesigner extends React.PureComponent> { 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() { diff --git a/core/src/client/IssuesListDesigner/IssuesListDesigner.tsx b/core/src/client/IssuesListDesigner/IssuesListDesigner.tsx index da294bfad17..d9dd54ba085 100644 --- a/core/src/client/IssuesListDesigner/IssuesListDesigner.tsx +++ b/core/src/client/IssuesListDesigner/IssuesListDesigner.tsx @@ -81,9 +81,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) => { diff --git a/core/src/client/ListDesigner/ListDesigner.tsx b/core/src/client/ListDesigner/ListDesigner.tsx index f3c0ad5da80..9f5022b3788 100644 --- a/core/src/client/ListDesigner/ListDesigner.tsx +++ b/core/src/client/ListDesigner/ListDesigner.tsx @@ -104,8 +104,8 @@ export class ListDesigner extends React.Component { navigate = async (returnUrlProvider: () => Promise, model?: ListModel): Promise => { 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 => { diff --git a/core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx b/core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx index e3b0e084b06..cb68d02ba79 100644 --- a/core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx +++ b/core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx @@ -137,9 +137,8 @@ class SampleTypeDesignerWrapper extends React.PureComponent { 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() { diff --git a/core/src/org/labkey/core/CoreController.java b/core/src/org/labkey/core/CoreController.java index acaf441e3aa..1feb49b9a91 100644 --- a/core/src/org/labkey/core/CoreController.java +++ b/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 _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 + { + @Override + public ActionURL getRedirectURL(ReturnUrlForm form) throws Exception + { + return form.getReturnActionURL(AppProps.getInstance().getHomePageActionURL()); + } + } } diff --git a/study/test/src/org/labkey/test/tests/study/AssayTest.java b/study/test/src/org/labkey/test/tests/study/AssayTest.java index 89a8f2bf686..a4186af26a2 100644 --- a/study/test/src/org/labkey/test/tests/study/AssayTest.java +++ b/study/test/src/org/labkey/test/tests/study/AssayTest.java @@ -29,6 +29,7 @@ import org.labkey.test.categories.Assays; import org.labkey.test.categories.Daily; import org.labkey.test.components.CustomizeView; +import org.labkey.test.components.DomainDesignerPage; import org.labkey.test.components.assay.AssayConstants; import org.labkey.test.components.domain.DomainFieldRow; import org.labkey.test.components.domain.DomainFormPanel; @@ -48,13 +49,13 @@ import org.labkey.test.util.TestDataGenerator; import org.labkey.test.util.data.TestDataUtils; -import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -69,6 +70,7 @@ public class AssayTest extends AbstractAssayTest private static final String ISSUE_53616_ASSAY = "Issue53616Assay"; private static final String ISSUE_53616_PROJECT = "Issue53616Project" + TRICKY_CHARACTERS_FOR_PROJECT_NAMES; private static final String ISSUE_53831_PROJECT = "Issue53831Project" + TRICKY_CHARACTERS_FOR_PROJECT_NAMES; + private static final String GH_1023_PROJECT = "GH1023Project" + TRICKY_CHARACTERS_FOR_PROJECT_NAMES; private static final String SAMPLE_FIELD_TEST_ASSAY = "SampleFieldTestAssay"; private static final String SAMPLE_FIELD_PROJECT_NAME = "Sample Field Test Project" + TRICKY_CHARACTERS_FOR_PROJECT_NAMES; @@ -90,6 +92,7 @@ protected void doCleanup(boolean afterTest) throws TestTimeoutException _containerHelper.deleteProject(ISSUE_53616_PROJECT, false); _containerHelper.deleteProject(ISSUE_53625_PROJECT, false); _containerHelper.deleteProject(ISSUE_53831_PROJECT, false); + _containerHelper.deleteProject(GH_1023_PROJECT, false); _userHelper.deleteUsers(false, TEST_ASSAY_USR_PI1, TEST_ASSAY_USR_TECH1); } @@ -1142,6 +1145,27 @@ private void verifyAssayImportForLookupValidator(String assayName, FieldInfo loo checker().verifyEquals("Lookup values not as expected.", List.of("123"), dataTable.getColumnDataAsText(lookupField.getLabel())); } + @Test // GitHub Issue #1023 + public void testNoExternalReturnUrlRedirect() throws Exception + { + _containerHelper.createProject(GH_1023_PROJECT, "Assay"); + goToProjectHome(GH_1023_PROJECT); + + // 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("assay", GH_1023_PROJECT, "designer", + Map.of("providerName", "General", "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)); + } + @Override protected BrowserType bestBrowser() { diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java index 202ceadbffa..d7c71b3687c 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java +++ b/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; @@ -784,4 +785,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)); + } } From fec8f76ba039e15bb5c3da5820f4164bc193329f Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Thu, 28 May 2026 16:55:51 -0700 Subject: [PATCH 3/5] GitHub Issue 1189: Update Google Analytics CSP (#7700) #### Rationale Google now documents their CSP requirements for using Google Analytics. Let's include everything they list, even if Firefox is the only browser that needs the extra server. #### Changes - Add `www.google.com` to `connect-src` --- .../src/org/labkey/core/analytics/AnalyticsServiceImpl.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/org/labkey/core/analytics/AnalyticsServiceImpl.java b/core/src/org/labkey/core/analytics/AnalyticsServiceImpl.java index 57acb0b3611..ac9c487d3fd 100644 --- a/core/src/org/labkey/core/analytics/AnalyticsServiceImpl.java +++ b/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); } } From cb8fb1cecea43444bb3139fd723c2a0e306a509c Mon Sep 17 00:00:00 2001 From: Nick Kerr Date: Tue, 2 Jun 2026 15:58:50 -0700 Subject: [PATCH 4/5] Query: remove defunct assertion (#7719) --- query/src/org/labkey/query/sql/QIfDefined.java | 1 - 1 file changed, 1 deletion(-) diff --git a/query/src/org/labkey/query/sql/QIfDefined.java b/query/src/org/labkey/query/sql/QIfDefined.java index 59553299738..ebb1d433bca 100644 --- a/query/src/org/labkey/query/sql/QIfDefined.java +++ b/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(); } From 1635a48bded9667087d7d651e1cf973ec1bbca25 Mon Sep 17 00:00:00 2001 From: Susan Hert Date: Tue, 2 Jun 2026 16:02:07 -0700 Subject: [PATCH 5/5] Fixing some more misc. accessibility issues (#7715) --- assay/package-lock.json | 8 ++++---- assay/package.json | 2 +- core/package-lock.json | 8 ++++---- core/package.json | 2 +- .../tests/devtools/AuthenticationProviderReorderTest.java | 4 ++-- experiment/package-lock.json | 8 ++++---- experiment/package.json | 2 +- pipeline/package-lock.json | 8 ++++---- pipeline/package.json | 2 +- 9 files changed, 22 insertions(+), 22 deletions(-) diff --git a/assay/package-lock.json b/assay/package-lock.json index c000076459a..232ab795b20 100644 --- a/assay/package-lock.json +++ b/assay/package-lock.json @@ -8,7 +8,7 @@ "name": "assay", "version": "0.0.0", "dependencies": { - "@labkey/components": "7.40.0" + "@labkey/components": "7.40.1" }, "devDependencies": { "@labkey/build": "9.1.4", @@ -3821,9 +3821,9 @@ } }, "node_modules/@labkey/components": { - "version": "7.40.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.40.0.tgz", - "integrity": "sha512-yOBkfRaW6O+mWakUrKlcCFSI/UADW+eHqJPufVZxDDtZhdbMS7o6slMX6nQ9VOb0UQIV2MCtDjPwvI/WUVrA3w==", + "version": "7.40.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.40.1.tgz", + "integrity": "sha512-SLPVdi/0zVlp9obXk7/AqVwmvixrB4mLUXNb0Me/T9HXx95vEADrz5axADFMXBquivNGpPvxeQOTaSGAlHLThw==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/assay/package.json b/assay/package.json index 7fa52aba408..c4d109111f1 100644 --- a/assay/package.json +++ b/assay/package.json @@ -55,7 +55,7 @@ } }, "dependencies": { - "@labkey/components": "7.40.0" + "@labkey/components": "7.40.1" }, "devDependencies": { "@labkey/build": "9.1.4", diff --git a/core/package-lock.json b/core/package-lock.json index 3e3f8f3d1e0..ae5e9a5b21f 100644 --- a/core/package-lock.json +++ b/core/package-lock.json @@ -8,7 +8,7 @@ "name": "labkey-core", "version": "0.0.0", "dependencies": { - "@labkey/components": "7.40.0", + "@labkey/components": "7.40.1", "@labkey/themes": "1.9.3" }, "devDependencies": { @@ -3824,9 +3824,9 @@ } }, "node_modules/@labkey/components": { - "version": "7.40.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.40.0.tgz", - "integrity": "sha512-yOBkfRaW6O+mWakUrKlcCFSI/UADW+eHqJPufVZxDDtZhdbMS7o6slMX6nQ9VOb0UQIV2MCtDjPwvI/WUVrA3w==", + "version": "7.40.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.40.1.tgz", + "integrity": "sha512-SLPVdi/0zVlp9obXk7/AqVwmvixrB4mLUXNb0Me/T9HXx95vEADrz5axADFMXBquivNGpPvxeQOTaSGAlHLThw==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/core/package.json b/core/package.json index fed2642e0b4..6098e1a5c3b 100644 --- a/core/package.json +++ b/core/package.json @@ -57,7 +57,7 @@ } }, "dependencies": { - "@labkey/components": "7.40.0", + "@labkey/components": "7.40.1", "@labkey/themes": "1.9.3" }, "devDependencies": { diff --git a/devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java b/devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java index 97eb094a9e4..5a16004fd77 100644 --- a/devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java +++ b/devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java @@ -51,7 +51,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(); @@ -64,7 +64,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(); } diff --git a/experiment/package-lock.json b/experiment/package-lock.json index 75f67f2cab1..dc5ea1b9375 100644 --- a/experiment/package-lock.json +++ b/experiment/package-lock.json @@ -8,7 +8,7 @@ "name": "experiment", "version": "0.0.0", "dependencies": { - "@labkey/components": "7.40.0" + "@labkey/components": "7.40.1" }, "devDependencies": { "@labkey/build": "9.1.4", @@ -3659,9 +3659,9 @@ } }, "node_modules/@labkey/components": { - "version": "7.40.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.40.0.tgz", - "integrity": "sha512-yOBkfRaW6O+mWakUrKlcCFSI/UADW+eHqJPufVZxDDtZhdbMS7o6slMX6nQ9VOb0UQIV2MCtDjPwvI/WUVrA3w==", + "version": "7.40.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.40.1.tgz", + "integrity": "sha512-SLPVdi/0zVlp9obXk7/AqVwmvixrB4mLUXNb0Me/T9HXx95vEADrz5axADFMXBquivNGpPvxeQOTaSGAlHLThw==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/experiment/package.json b/experiment/package.json index d4208d502a0..d2d8d310296 100644 --- a/experiment/package.json +++ b/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", diff --git a/pipeline/package-lock.json b/pipeline/package-lock.json index d0d957c8395..6ca8952d036 100644 --- a/pipeline/package-lock.json +++ b/pipeline/package-lock.json @@ -8,7 +8,7 @@ "name": "pipeline", "version": "0.0.0", "dependencies": { - "@labkey/components": "7.40.0" + "@labkey/components": "7.40.1" }, "devDependencies": { "@labkey/build": "9.1.4", @@ -2914,9 +2914,9 @@ } }, "node_modules/@labkey/components": { - "version": "7.40.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.40.0.tgz", - "integrity": "sha512-yOBkfRaW6O+mWakUrKlcCFSI/UADW+eHqJPufVZxDDtZhdbMS7o6slMX6nQ9VOb0UQIV2MCtDjPwvI/WUVrA3w==", + "version": "7.40.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.40.1.tgz", + "integrity": "sha512-SLPVdi/0zVlp9obXk7/AqVwmvixrB4mLUXNb0Me/T9HXx95vEADrz5axADFMXBquivNGpPvxeQOTaSGAlHLThw==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/pipeline/package.json b/pipeline/package.json index 47ae736c4fb..5f235e99f74 100644 --- a/pipeline/package.json +++ b/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",