From 33346e8629ffb77059a8461d103fe49dd39e019e Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 19 Sep 2026 15:05:12 -0700 Subject: [PATCH 1/7] Deprecate LabKeyTransformSessionId cookie auth --- .../assay/transform/DataTransformService.java | 6 ++++ .../labkey/api/security/SecurityManager.java | 31 +++++++++++++------ core/src/org/labkey/core/CoreModule.java | 4 +++ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/api/src/org/labkey/api/assay/transform/DataTransformService.java b/api/src/org/labkey/api/assay/transform/DataTransformService.java index 4e421e95efc..1c35491ca9b 100644 --- a/api/src/org/labkey/api/assay/transform/DataTransformService.java +++ b/api/src/org/labkey/api/assay/transform/DataTransformService.java @@ -312,6 +312,12 @@ private boolean isDefault(ExpProtocol protocol) */ private String getSessionInfo(@Nullable HttpServletRequest request, String apiKey) { + if (request == null) + { + // GH Issue 1489: background/pipeline jobs have no live HTTP session, so use apikey authentication + // directly instead of the deprecated LabKeyTransformSessionId cookie. + return "labkey.setDefaults(apiKey = \"" + apiKey + "\")\n"; + } return "labkey.sessionCookieName = \"" + getSessionCookieName(request) + "\"\n" + "labkey.sessionCookieContents = \"" + getSessionId(request, apiKey) + "\"\n"; } diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index 26a7266ee15..3bc71632aa6 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -174,6 +174,8 @@ public class SecurityManager static final String CIRCULAR_GROUP_ERROR_MESSAGE = "Can't add a group that results in a circular group relation"; public static final String TRANSFORM_SESSION_ID = "LabKeyTransformSessionId"; // issue 19748 + /** GH Issue 1489: gates acceptance of the deprecated TRANSFORM_SESSION_ID cookie/parameter; default off */ + public static final String FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID = "AllowTransformSessionIdAuth"; public static final String API_KEY = "apikey"; public static final String USER_ID_KEY = User.class.getName() + "$userId"; @@ -672,7 +674,7 @@ public static Pair attemptAuthentication(HttpServletRe // Passing via the "apikey" HTTP header is our preferred approach and used by most // LabKey client API implementations String apiKey = request.getHeader(API_KEY); - + if (null == apiKey) { String authorization = request.getHeader("Authorization"); @@ -682,14 +684,11 @@ public static Pair attemptAuthentication(HttpServletRe if (null == apiKey) { - // Issue 40482: Deprecate using 'LabKeyTransformSessionId' in preference for 'apikey' authentication + // Issue 40482 / GH Issue 1489: Deprecate using 'LabKeyTransformSessionId' in preference for 'apikey' authentication // issue 19748: need alternative to JSESSIONID for pipeline job transform script usage - apiKey = PageFlowUtil.getCookieValue(request.getCookies(), TRANSFORM_SESSION_ID, null); - if (null != apiKey) - { - AUTH_LOG.warn("Using '" + TRANSFORM_SESSION_ID + "' cookie for authentication is deprecated; use 'apikey' instead"); - } - else + String transformSessionId = PageFlowUtil.getCookieValue(request.getCookies(), TRANSFORM_SESSION_ID, null); + + if (null == transformSessionId) { // Support as a GET parameter as well, not just as a cookie, to support authentication // through SSRS which can't be made to use BasicAuth, pass cookies, or other HTTP headers. @@ -697,13 +696,27 @@ public static Pair attemptAuthentication(HttpServletRe try { Map params = PageFlowUtil.mapFromQueryString(request.getQueryString()); - apiKey = params.get(TRANSFORM_SESSION_ID); + transformSessionId = params.get(TRANSFORM_SESSION_ID); } catch (IllegalArgumentException e) { throw new UnsupportedEncodingException(e.getMessage()); } } + + if (null != transformSessionId) + { + if (AppProps.getInstance().isOptionalFeatureEnabled(FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID)) + { + apiKey = transformSessionId; + AUTH_LOG.warn("Using '" + TRANSFORM_SESSION_ID + "' cookie/parameter for authentication is deprecated; use 'apikey' instead"); + } + else + { + AUTH_LOG.warn("Rejected deprecated '" + TRANSFORM_SESSION_ID + "' cookie/parameter authentication attempt; enable the '" + + FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID + "' feature flag temporarily, or switch the script to 'apikey' authentication"); + } + } } return null != apiKey ? Pair.of(API_KEY, apiKey) : null; diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index d8fc2e62205..a50b01d25a8 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -536,6 +536,10 @@ public QuerySchema createSchema(DefaultSchema schema, Module module) "Notifications 'inbox' count display in the header bar with click to show the notifications panel of unread notifications.", false, true); OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(SQLFragment.FEATUREFLAG_DISABLE_STRICT_CHECKS, "Disable SQLFragment strict checks", "Disables strict SQL generation safeguards in SQLFragment.appendIdentifier and QueryPivot value emission", false, true, FeatureType.Deprecated)); + OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(SecurityManager.FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID, + "Allow 'LabKeyTransformSessionId' cookie/parameter authentication", + "Allows pipeline/transform scripts to authenticate via the legacy 'LabKeyTransformSessionId' cookie or query parameter instead of 'apikey' authentication. This option will be removed in a future release of LabKey Server.", + false, false, FeatureType.Deprecated)); OptionalFeatureService.get().addExperimentalFeatureFlag(PageTemplate.EXPERIMENTAL_SHORT_CIRCUIT_ROBOTS, "Short-circuit robots", "Save resources by not rendering pages marked as 'noindex' for robots. This is experimental as not all robots are search engines.", From bbece3f0a3d03ba8c33a6fe95859ac92cc3f1adb Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 21 Sep 2026 13:31:12 -0700 Subject: [PATCH 2/7] Fix NPE in issues hasAdminPermission() --- .../src/org/labkey/experiment/api/ExpRunItemTableImpl.java | 5 +++-- issues/src/org/labkey/issue/IssuesController.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpRunItemTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunItemTableImpl.java index 9bec894be7b..ab0cbe5bb78 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunItemTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunItemTableImpl.java @@ -41,6 +41,7 @@ import org.labkey.api.query.LookupForeignKey; import org.labkey.api.query.QueryKey; import org.labkey.api.query.UserSchema; +import org.labkey.api.query.UserSchema.HasContextualRoles; import org.labkey.api.security.User; import org.labkey.api.security.UserPrincipal; import org.labkey.api.security.permissions.Permission; @@ -75,9 +76,9 @@ public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class getReadOnlyFields(Issue.action action) protected boolean hasAdminPermission(User user, IssueObject issue) { return getContainer().hasPermission(user, AdminPermission.class, - (issue.getCreatedBy() == user.getUserId() ? RoleManager.roleSet(OwnerRole.class) : null)); + (issue.getCreatedBy() == user.getUserId() ? RoleManager.roleSet(OwnerRole.class) : Set.of())); } public CustomColumnConfiguration getColumnConfiguration() From c95335749e1fce02d513b1f13fa7ff94986a378d Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 21 Sep 2026 14:00:57 -0700 Subject: [PATCH 3/7] Limit deprecation to "LabKeyTransformSessionId" cookie; leave support for parameter. --- .../labkey/api/security/SecurityManager.java | 46 +++++++++---------- core/src/org/labkey/core/CoreModule.java | 4 +- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index c6573e8dd01..f73c1ae2b1c 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -664,15 +664,15 @@ public static Pair attemptAuthentication(HttpServletRe } /** - * Determine if an API key is present, checking "apikey" header first and then the special "transform" cookie and - * parameters. Return a pair with the API key if it's present; otherwise return null. + * Determine if an API key is present, checking "apikey" header first and then the special "transform" parameter + * supported only for SSRS. Return a pair with the API key if it's present; otherwise return null. * @param request Current request * @return First API key found or null if an apikey is not present. */ private static @Nullable Pair getApiKey(HttpServletRequest request) throws UnsupportedEncodingException { - // Passing via the "apikey" HTTP header is our preferred approach and used by most - // LabKey client API implementations + // Passing via the "apikey" HTTP header is our preferred approach and used by most LabKey client API + // implementations String apiKey = request.getHeader(API_KEY); if (null == apiKey) @@ -684,37 +684,37 @@ public static Pair attemptAuthentication(HttpServletRe if (null == apiKey) { - // Issue 40482 / GH Issue 1489: Deprecate using 'LabKeyTransformSessionId' in preference for 'apikey' authentication - // issue 19748: need alternative to JSESSIONID for pipeline job transform script usage + // Issue 40482 / GH Issue 1489: Deprecate using 'LabKeyTransformSessionId' cookie in preference for 'apikey' + // header authentication. String transformSessionId = PageFlowUtil.getCookieValue(request.getCookies(), TRANSFORM_SESSION_ID, null); - if (null == transformSessionId) + if (null != transformSessionId) { - // Support as a GET parameter as well, not just as a cookie, to support authentication - // through SSRS which can't be made to use BasicAuth, pass cookies, or other HTTP headers. - // Do not use request.getParameter() since that will consume the POST body, #32711. - try + if (AppProps.getInstance().isOptionalFeatureEnabled(FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID)) { - Map params = PageFlowUtil.mapFromQueryString(request.getQueryString()); - transformSessionId = params.get(TRANSFORM_SESSION_ID); + apiKey = transformSessionId; + AUTH_LOG.warn("Using '" + TRANSFORM_SESSION_ID + "' cookie for authentication is deprecated; use 'apikey' header instead"); } - catch (IllegalArgumentException e) + else { - throw new UnsupportedEncodingException(e.getMessage()); + AUTH_LOG.warn("Rejected deprecated \"" + TRANSFORM_SESSION_ID + "\" cookie authentication attempt; " + + "enable the \"Allow 'LabKeyTransformSessionId' cookie authentication\" feature flag temporarily, " + + "or switch the script to 'apikey' header authentication."); } } - - if (null != transformSessionId) + else { - if (AppProps.getInstance().isOptionalFeatureEnabled(FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID)) + // Continue to support "LabKeyTransformSessionId" as a GET parameter, to support authentication through + // SSRS which can't be made to use BasicAuth, pass cookies, or other HTTP headers. Do not use + // request.getParameter() since that will consume the POST body, #32711. + try { - apiKey = transformSessionId; - AUTH_LOG.warn("Using '" + TRANSFORM_SESSION_ID + "' cookie/parameter for authentication is deprecated; use 'apikey' instead"); + Map params = PageFlowUtil.mapFromQueryString(request.getQueryString()); + apiKey = params.get(TRANSFORM_SESSION_ID); } - else + catch (IllegalArgumentException e) { - AUTH_LOG.warn("Rejected deprecated '" + TRANSFORM_SESSION_ID + "' cookie/parameter authentication attempt; enable the '" + - FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID + "' feature flag temporarily, or switch the script to 'apikey' authentication"); + throw new UnsupportedEncodingException(e.getMessage()); } } } diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index a50b01d25a8..3046b94562f 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -537,8 +537,8 @@ public QuerySchema createSchema(DefaultSchema schema, Module module) OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(SQLFragment.FEATUREFLAG_DISABLE_STRICT_CHECKS, "Disable SQLFragment strict checks", "Disables strict SQL generation safeguards in SQLFragment.appendIdentifier and QueryPivot value emission", false, true, FeatureType.Deprecated)); OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(SecurityManager.FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID, - "Allow 'LabKeyTransformSessionId' cookie/parameter authentication", - "Allows pipeline/transform scripts to authenticate via the legacy 'LabKeyTransformSessionId' cookie or query parameter instead of 'apikey' authentication. This option will be removed in a future release of LabKey Server.", + "Allow 'LabKeyTransformSessionId' cookie authentication", + "Allows pipeline/transform scripts to authenticate via the legacy 'LabKeyTransformSessionId' cookie instead of 'apikey' authentication. This option will be removed in a future release of LabKey Server.", false, false, FeatureType.Deprecated)); OptionalFeatureService.get().addExperimentalFeatureFlag(PageTemplate.EXPERIMENTAL_SHORT_CIRCUIT_ROBOTS, "Short-circuit robots", From 61e1b619a3bbd36331ad93302e5fd188173da971 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 21 Sep 2026 14:19:09 -0700 Subject: [PATCH 4/7] Return records instead of pairs for clarity --- .../org/labkey/api/security/AuthFilter.java | 10 ++--- .../labkey/api/security/SecurityManager.java | 44 +++++++++++-------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthFilter.java b/api/src/org/labkey/api/security/AuthFilter.java index f1f9a29cf72..94d5aa87fef 100644 --- a/api/src/org/labkey/api/security/AuthFilter.java +++ b/api/src/org/labkey/api/security/AuthFilter.java @@ -30,6 +30,7 @@ import org.labkey.api.module.ModuleLoader; import org.labkey.api.module.SafeFlushResponseWrapper; import org.labkey.api.query.QueryService; +import org.labkey.api.security.SecurityManager.AuthenticationAttempt; import org.labkey.api.security.impersonation.ImpersonationContextFactory; import org.labkey.api.security.impersonation.UnauthorizedImpersonationException; import org.labkey.api.settings.AppProps; @@ -39,7 +40,6 @@ import org.labkey.api.util.GUID; import org.labkey.api.util.HttpUtil; import org.labkey.api.util.HttpsUtil; -import org.labkey.api.util.Pair; import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewServlet; @@ -168,12 +168,12 @@ else if (!AppProps.getInstance().isDevMode()) try { - Pair pair = SecurityManager.attemptAuthentication(req, resp); + AuthenticationAttempt attempt = SecurityManager.attemptAuthentication(req, resp); - if (null != pair) + if (null != attempt) { - user = pair.getKey(); - req = pair.getValue(); + user = attempt.user(); + req = attempt.request(); } } catch (UnauthorizedImpersonationException uie) diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index f73c1ae2b1c..fb284233812 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -174,7 +174,7 @@ public class SecurityManager static final String CIRCULAR_GROUP_ERROR_MESSAGE = "Can't add a group that results in a circular group relation"; public static final String TRANSFORM_SESSION_ID = "LabKeyTransformSessionId"; // issue 19748 - /** GH Issue 1489: gates acceptance of the deprecated TRANSFORM_SESSION_ID cookie/parameter; default off */ + /** GH Issue 1489: gates acceptance of the deprecated TRANSFORM_SESSION_ID cookie; default off */ public static final String FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID = "AllowTransformSessionIdAuth"; public static final String API_KEY = "apikey"; @@ -419,11 +419,13 @@ public void userAccountDisabled(User user) } } - private static @Nullable Pair getBasicCredentials(HttpServletRequest request) + private record Credentials(String username, String password) {} + + private static @Nullable Credentials getBasicCredentials(HttpServletRequest request) { // Authorization: Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ== String authorization = request.getHeader("Authorization"); - Pair ret = null; + Credentials ret = null; if (null != authorization && authorization.startsWith("Basic")) { @@ -436,19 +438,19 @@ public void userAccountDisabled(User user) String username = auth.substring(0, colon); String password = auth.substring(colon+1); - ret = new Pair<>(username, password); + ret = new Credentials(username, password); } return ret; } // Authorization: Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ== - private static @Nullable User authenticateBasic(HttpServletRequest request, @NotNull Pair basicCredentials) + private static @Nullable User authenticateBasic(HttpServletRequest request, @NotNull Credentials basicCredentials) { try { - String rawEmail = basicCredentials.getKey(); - String password = basicCredentials.getValue(); + String rawEmail = basicCredentials.username(); + String password = basicCredentials.password(); if (rawEmail.equalsIgnoreCase("guest")) return AuthFilter.getGuestUser(); @@ -533,29 +535,31 @@ public static User getSessionUser(HandshakeRequest request) return sessionUser; } - public static Pair attemptAuthentication(HttpServletRequest request, HttpServletResponse response) throws UnsupportedEncodingException + public record AuthenticationAttempt(User user, HttpServletRequest request) {} + + public static @Nullable AuthenticationAttempt attemptAuthentication(HttpServletRequest request, HttpServletResponse response) throws UnsupportedEncodingException { AUTH_LOG.debug("Starting authentication attempt via session, Basic auth, or API key header for request \"{}\"", request.getRequestURI()); // Current best practice is to pass API keys via an "apikey" header, but they can be passed via basic auth // (username "apikey"), supported for backwards compatibility and clients that don't support custom headers. - @Nullable Pair basicCredentials = getBasicCredentials(request); - AUTH_LOG.debug(" {}", null == basicCredentials ? "Basic auth credentials not provided" : "Basic auth credentials provided: " + basicCredentials.getKey() + " and " + basicCredentials.getValue().length() + " character password"); + @Nullable Credentials basicCredentials = getBasicCredentials(request); + AUTH_LOG.debug(" {}", null == basicCredentials ? "Basic auth credentials not provided" : "Basic auth credentials provided: " + basicCredentials.username() + " and " + basicCredentials.password().length() + " character password"); if (null == basicCredentials) { basicCredentials = getApiKey(request); - AUTH_LOG.debug(" {}", null == basicCredentials ? "API key not provided" : "API key provided: " + basicCredentials.getKey() + " and " + basicCredentials.getValue().length() + " character key"); + AUTH_LOG.debug(" {}", null == basicCredentials ? "API key not provided" : "API key provided: " + basicCredentials.username() + " and " + basicCredentials.password().length() + " character key"); } // Handle session API key early, if present and valid if (basicCredentials != null) { - String username = basicCredentials.first; + String username = basicCredentials.username(); if (API_KEY.equals(username)) { - String apiKey = basicCredentials.second; + String apiKey = basicCredentials.password(); HttpSession session = SessionApiKeyManager.get().getContext(apiKey); if (null != session) @@ -641,7 +645,7 @@ public static Pair attemptAuthentication(HttpServletRe AUTH_LOG.debug(" Basic authentication succeeded: {}", u); request.setAttribute(AUTHENTICATION_METHOD, "Basic"); // accept Guest as valid credentials from authenticateBasic() - return new Pair<>(u, request); + return new AuthenticationAttempt(u, request); } else { @@ -655,7 +659,7 @@ public static Pair attemptAuthentication(HttpServletRe // u = AuthenticationManager.attemptRequestAuthentication(request); // } - return null == u || u.isGuest() ? null : new Pair<>(u, request); + return null == u || u.isGuest() ? null : new AuthenticationAttempt(u, request); } finally { @@ -664,12 +668,14 @@ public static Pair attemptAuthentication(HttpServletRe } /** - * Determine if an API key is present, checking "apikey" header first and then the special "transform" parameter - * supported only for SSRS. Return a pair with the API key if it's present; otherwise return null. + * Determine if an API key is present, checking the "apikey" header first, then the deprecated + * "LabKeyTransformSessionId" cookie (gated behind {@link #FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID}), and finally + * the "LabKeyTransformSessionId" GET parameter (supported permanently, since SSRS can't be made to use the header + * or a cookie). Return the credentials if an API key is present via any of these; otherwise return null. * @param request Current request * @return First API key found or null if an apikey is not present. */ - private static @Nullable Pair getApiKey(HttpServletRequest request) throws UnsupportedEncodingException + private static @Nullable Credentials getApiKey(HttpServletRequest request) throws UnsupportedEncodingException { // Passing via the "apikey" HTTP header is our preferred approach and used by most LabKey client API // implementations @@ -719,7 +725,7 @@ public static Pair attemptAuthentication(HttpServletRe } } - return null != apiKey ? Pair.of(API_KEY, apiKey) : null; + return null != apiKey ? new Credentials(API_KEY, apiKey) : null; } public static final int SECONDS_PER_DAY = 60*60*24; From c551cd01665abd1f0ef8b7080575958cb4c7716d Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 21 Sep 2026 16:00:16 -0700 Subject: [PATCH 5/7] Deprecate more legacy substitution parameters --- .../assay/transform/DataTransformService.java | 14 +++++++++----- .../labkey/api/security/SecurityManager.java | 2 +- core/src/org/labkey/core/CoreModule.java | 5 ++--- core/src/org/labkey/core/Reports.md | 2 +- .../pipeline/api/SimpleTaskFactory.java | 19 ++++++++----------- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/api/src/org/labkey/api/assay/transform/DataTransformService.java b/api/src/org/labkey/api/assay/transform/DataTransformService.java index 1c35491ca9b..cd6bc298a9d 100644 --- a/api/src/org/labkey/api/assay/transform/DataTransformService.java +++ b/api/src/org/labkey/api/assay/transform/DataTransformService.java @@ -48,6 +48,8 @@ import java.util.Map; import java.util.Set; +import static org.labkey.api.security.SecurityManager.FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID; + public class DataTransformService { private static final DataTransformService _instance = new DataTransformService(); @@ -257,12 +259,14 @@ public void addStandardParameters(@Nullable HttpServletRequest request, @Nullabl if (srcDir != null && srcDir.exists()) paramMap.put(SRC_DIR_REPLACEMENT, srcDir.toNioPathForRead().toFile().getAbsolutePath().replaceAll("\\\\", "/")); } - paramMap.put(R_SESSIONID_REPLACEMENT, getSessionInfo(request, apiKey)); - paramMap.put(LEGACY_SESSION_COOKIE_NAME_REPLACEMENT, getSessionCookieName(request)); - paramMap.put(LEGACY_SESSION_ID_REPLACEMENT, getSessionId(request, apiKey)); + if (AppProps.getInstance().isOptionalFeatureEnabled(FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID)) + { + paramMap.put(R_SESSIONID_REPLACEMENT, getSessionInfo(request, apiKey)); + paramMap.put(LEGACY_SESSION_COOKIE_NAME_REPLACEMENT, getSessionCookieName(request)); + paramMap.put(LEGACY_SESSION_ID_REPLACEMENT, getSessionId(request, apiKey)); + } paramMap.put(SecurityManager.API_KEY, apiKey); - paramMap.put(BASE_SERVER_URL_REPLACEMENT, AppProps.getInstance().getBaseServerUrl() - + AppProps.getInstance().getContextPath()); + paramMap.put(BASE_SERVER_URL_REPLACEMENT, AppProps.getInstance().getBaseServerUrl() + AppProps.getInstance().getContextPath()); paramMap.put(CONTAINER_PATH, container == null ? null : container.getPath()); } diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index fb284233812..d53f13abb2c 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -704,7 +704,7 @@ public record AuthenticationAttempt(User user, HttpServletRequest request) {} else { AUTH_LOG.warn("Rejected deprecated \"" + TRANSFORM_SESSION_ID + "\" cookie authentication attempt; " + - "enable the \"Allow 'LabKeyTransformSessionId' cookie authentication\" feature flag temporarily, " + + "enable the \"Allow script authentication via legacy substitution parameters\" feature flag temporarily, " + "or switch the script to 'apikey' header authentication."); } } diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 3046b94562f..6c769b90071 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -78,7 +78,6 @@ import org.labkey.api.data.dialect.BasePostgreSqlDialect; import org.labkey.api.data.dialect.PostgreSqlService; import org.labkey.api.data.dialect.SqlDialect; -import org.labkey.api.data.dialect.SqlDialect.DataSourcePropertyReader; import org.labkey.api.data.dialect.SqlDialectManager; import org.labkey.api.data.dialect.SqlDialectRegistry; import org.labkey.api.data.statistics.StatsService; @@ -537,8 +536,8 @@ public QuerySchema createSchema(DefaultSchema schema, Module module) OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(SQLFragment.FEATUREFLAG_DISABLE_STRICT_CHECKS, "Disable SQLFragment strict checks", "Disables strict SQL generation safeguards in SQLFragment.appendIdentifier and QueryPivot value emission", false, true, FeatureType.Deprecated)); OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(SecurityManager.FEATUREFLAG_ALLOW_TRANSFORM_SESSION_ID, - "Allow 'LabKeyTransformSessionId' cookie authentication", - "Allows pipeline/transform scripts to authenticate via the legacy 'LabKeyTransformSessionId' cookie instead of 'apikey' authentication. This option will be removed in a future release of LabKey Server.", + "Allow script authentication via legacy substitution parameters", + "Allows pipeline/transform scripts to authenticate via legacy approaches ('LabKeyTransformSessionId', 'rLabkeySessionId', 'httpSessionId', and 'sessionCookieName' substitution parameters) instead of 'apikey' header authentication. This option will be removed in a future release of LabKey Server.", false, false, FeatureType.Deprecated)); OptionalFeatureService.get().addExperimentalFeatureFlag(PageTemplate.EXPERIMENTAL_SHORT_CIRCUIT_ROBOTS, "Short-circuit robots", diff --git a/core/src/org/labkey/core/Reports.md b/core/src/org/labkey/core/Reports.md index 6f3f1b1d5c8..7acbe28180b 100644 --- a/core/src/org/labkey/core/Reports.md +++ b/core/src/org/labkey/core/Reports.md @@ -131,7 +131,7 @@ The older bare inline form (`${id:name}` with no leading `#`) still works but is Use `regex(...)` inside a token — e.g. `${fileout:regex(.*?\.gct)}` — when the script generates files whose exact names aren't known ahead of time; LabKey maps any file matching the pattern to that output slot. -A separate set of tokens is substituted both into the engine invocation command line *and* — if you reference them directly — into the script body itself, since both substitution passes share the same replacement map: `${scriptName}`, `${scriptFile}`, `${workingDir}`, `${apikey}`, `${rLabkeySessionId}`, `${httpSessionId}`, `${sessionCookieName}`, `${baseServerURL}`, `${containerPath}`. +A separate set of tokens is substituted both into the engine invocation command line *and* — if you reference them directly — into the script body itself, since both substitution passes share the same replacement map: `${scriptName}`, `${scriptFile}`, `${workingDir}`, `${apikey}`, `${baseServerURL}`, `${containerPath}`. **`${srcDirectory}` does not work for Reports** despite being defined alongside this family — it's only ever populated for assay *transform* scripts, a different feature. If you reference it in a report script (e.g. `source("${srcDirectory}/util.R")`), it will not resolve, and — unlike the command-line substitution pass, which silently strips unmatched tokens — the script-body substitution pass writes it out **verbatim**, so the script fails at runtime trying to open a file literally named `${srcDirectory}/...`. Don't use it when porting a script into an R report. diff --git a/pipeline/src/org/labkey/pipeline/api/SimpleTaskFactory.java b/pipeline/src/org/labkey/pipeline/api/SimpleTaskFactory.java index 25d326aae77..0a32c1a97e9 100644 --- a/pipeline/src/org/labkey/pipeline/api/SimpleTaskFactory.java +++ b/pipeline/src/org/labkey/pipeline/api/SimpleTaskFactory.java @@ -62,9 +62,6 @@ import java.util.Set; /** - * User: kevink - * Date: 11/18/13 - * * SimpleTaskFactory is a base class for creating file-based module task definitions. * Modules register a XMLBean SchemaType with a XMLBeanTaskFactoryFactory to create concrete TaskFactory types. * CONSIDER: Move to API or Internal so other modules can create subclasses. @@ -75,14 +72,14 @@ public abstract class SimpleTaskFactory extends CommandTaskImpl.Factory { protected static Set RESERVED_TOKENS = new CaseInsensitiveHashSet( - PipelineJob.PIPELINE_JOB_INFO_PARAM, - PipelineJob.PIPELINE_TASK_INFO_PARAM, - PipelineJob.PIPELINE_TASK_OUTPUT_PARAMS_PARAM, - // The following replacements aren't used yet, but are reserved for future use. - DataTransformService.RUN_INFO_REPLACEMENT, - DataTransformService.SRC_DIR_REPLACEMENT, - DataTransformService.R_SESSIONID_REPLACEMENT - ); + PipelineJob.PIPELINE_JOB_INFO_PARAM, + PipelineJob.PIPELINE_TASK_INFO_PARAM, + PipelineJob.PIPELINE_TASK_OUTPUT_PARAMS_PARAM, + // The following replacements aren't used yet, but are reserved for future use. + DataTransformService.RUN_INFO_REPLACEMENT, + DataTransformService.SRC_DIR_REPLACEMENT, + DataTransformService.R_SESSIONID_REPLACEMENT + ); protected Map _params; From 8b53fdb2cd3e7e558fad1cceb3e9be16207b15f9 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 22 Sep 2026 07:52:47 -0700 Subject: [PATCH 6/7] Clear warnings --- api/src/org/labkey/api/pipeline/TaskFactory.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/pipeline/TaskFactory.java b/api/src/org/labkey/api/pipeline/TaskFactory.java index 7c23aed63b0..b0ac807f129 100644 --- a/api/src/org/labkey/api/pipeline/TaskFactory.java +++ b/api/src/org/labkey/api/pipeline/TaskFactory.java @@ -17,6 +17,7 @@ import org.apache.logging.log4j.Logger; import org.labkey.api.module.Module; +import org.labkey.api.pipeline.PipelineJob.Task; import org.labkey.api.pipeline.file.FileAnalysisJobSupport; import org.labkey.api.util.FileType; @@ -27,8 +28,6 @@ * TaskFactory is responsible for creating a task to run on a * PipelineJob. Create an implementation of this interface to support custom * Task configuration inside the Mule configuration Spring context. - * - * @author brendanx */ public interface TaskFactory { @@ -36,17 +35,17 @@ public interface TaskFactory TaskId getActiveId(PipelineJob job); - PipelineJob.Task createTask(PipelineJob job); + Task createTask(PipelineJob job); - TaskFactory cloneAndConfigure(SettingsType settings) throws CloneNotSupportedException; + TaskFactory cloneAndConfigure(SettingsType settings) throws CloneNotSupportedException; /** @return the types of files that are consumable by this task as input */ List getInputTypes(); /** - * All of the ProtocolAction names that this task may include when it runs. It need not execute all of them for - * each invocation. - * These names are used to build up a full Experiment Protocol for each pipeline to which this tasks belongs. + * All the ProtocolAction names that this task may include when it runs. It need not execute all of them for + * each invocation. These names are used to build up a full Experiment Protocol for each pipeline to which this + * task belongs. */ List getProtocolActionNames(); @@ -57,7 +56,7 @@ public interface TaskFactory String getGroupParameterName(); /** - * @return true if this task operates on all of the split items (say, multiple input files) as a whole, or false + * @return true if this task operates on all the split items (say, multiple input files) as a whole, or false * if each split item should be operated on independently (and potentially in parallel) */ boolean isJoin(); From 544e7901cb616b5d350ab917e57e07eafce69795 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 22 Sep 2026 11:09:34 -0700 Subject: [PATCH 7/7] Fail fast if script contains unreplaced substitutions --- api/src/org/labkey/api/ApiModule.java | 2 + .../api/reports/ExternalScriptEngine.java | 79 ++++++++++++++++++- .../api/reports/report/r/RScriptEngine.java | 2 +- 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index b687e91f753..01a1fc0afef 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -124,6 +124,7 @@ import org.labkey.api.reader.MapLoader; import org.labkey.api.reader.StrictBoundedReader; import org.labkey.api.reader.TabLoader; +import org.labkey.api.reports.ExternalScriptEngine; import org.labkey.api.reports.model.ViewCategoryManager; import org.labkey.api.reports.report.ReportType; import org.labkey.api.reports.report.r.RReport; @@ -440,6 +441,7 @@ public void registerServlets(ServletContext servletCtx) ExcelWriter.TestCase.class, ExistingRecordDataIterator.TestCase.class, ExperimentJSONConverter.TestCase.class, + ExternalScriptEngine.TestCase.class, ExtUtil.TestCase.class, FieldKey.TestCase.class, FileType.TestCase.class, diff --git a/api/src/org/labkey/api/reports/ExternalScriptEngine.java b/api/src/org/labkey/api/reports/ExternalScriptEngine.java index 5d35bccdbd8..737ed9f69b5 100644 --- a/api/src/org/labkey/api/reports/ExternalScriptEngine.java +++ b/api/src/org/labkey/api/reports/ExternalScriptEngine.java @@ -19,6 +19,10 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; import org.labkey.api.miniprofiler.CustomTiming; import org.labkey.api.miniprofiler.MiniProfiler; import org.labkey.api.pipeline.PipelineJobService; @@ -39,6 +43,7 @@ import javax.script.ScriptEngineFactory; import javax.script.ScriptException; import javax.script.SimpleBindings; +import javax.script.SimpleScriptContext; import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.File; @@ -49,8 +54,10 @@ import java.io.Writer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -59,6 +66,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.labkey.api.reports.report.r.ParamReplacementSvc.SubstitutionSyntax.INLINE; + /* * User: Karl Lum * Date: Dec 2, 2008 @@ -146,7 +155,7 @@ protected Object evalScript(String script, ScriptContext context) throws ScriptE * Prepare the on-disk script file that will be executed. The default writes the script as-is; subclasses (e.g. the * R engine's knitr handling) may wrap it in a different driver script. */ - protected FileLike prepareScriptFile(String script, ScriptContext context, List extensions) + protected FileLike prepareScriptFile(String script, ScriptContext context, List extensions) throws ScriptException { return writeScriptFile(script, context, extensions); } @@ -552,7 +561,7 @@ protected int runProcess(ScriptContext context, LabKeyProcessBuilder pb, StringB } } - protected FileLike writeScriptFile(String script, ScriptContext context, List extensions) + protected FileLike writeScriptFile(String script, ScriptContext context, List extensions) throws ScriptException { // write out the script file to disk using the first extension as the default FileLike scriptFile; @@ -590,12 +599,25 @@ protected FileLike writeScriptFile(String script, ScriptContext context, List unreplaced = new LinkedHashSet<>(); + while (matcher.find()) + unreplaced.add(matcher.group(1)); + + if (!unreplaced.isEmpty()) + throw new ScriptException("Unreplaced substitution parameter(s) found in script: " + String.join(", ", unreplaced)); + try (PrintWriter pw = new PrintWriter(new BufferedWriter(new OutputStreamWriter(scriptFile.openOutputStream(), StandardCharsets.UTF_8)))) { pw.write(script); } } } + catch (ScriptException e) + { + throw e; + } catch (Exception e) { ExceptionUtil.logExceptionToMothership(null, e); @@ -705,4 +727,57 @@ public boolean supportsContext(LabKeyScriptEngineManager.EngineContext context) { return true; } + + public static class TestCase extends Assert + { + private static final String KNOWN_PARAM = "knownParam"; + private static final String KNOWN_VALUE = "replacement value"; + + private ExternalScriptEngine _engine; + private ScriptContext _context; + private FileLike _scriptFile; + + @Before + public void setUp() + { + _engine = new ExternalScriptEngine(null); + _context = new SimpleScriptContext(); + Bindings bindings = _engine.createBindings(); + bindings.put(PARAM_REPLACEMENT_MAP, Map.of(KNOWN_PARAM, KNOWN_VALUE)); + _context.setBindings(bindings, ScriptContext.ENGINE_SCOPE); + } + + @After + public void tearDown() throws IOException + { + if (null != _scriptFile && _scriptFile.exists()) + _scriptFile.delete(); + } + + @Test + public void testFullyReplacedScriptSucceeds() throws ScriptException + { + String script = "print(\"${" + KNOWN_PARAM + "}\")"; + _scriptFile = _engine.writeScriptFile(script, _context, List.of("R")); + assertTrue("Script file should have been written", _scriptFile.exists()); + } + + @Test + public void testUnreplacedSubstitutionThrows() + { + String script = "print(\"${unknownParam}\")"; + ScriptException e = assertThrows(ScriptException.class, () -> _engine.writeScriptFile(script, _context, List.of("R"))); + assertTrue("Exception message should name the unreplaced parameter", e.getMessage().contains("unknownParam")); + } + + @Test + public void testMultipleUnreplacedSubstitutionsAreAllNamed() + { + String script = "${firstUnknown} and ${" + KNOWN_PARAM + "} and ${secondUnknown}"; + ScriptException e = assertThrows(ScriptException.class, () -> _engine.writeScriptFile(script, _context, List.of("R"))); + assertTrue("Exception message should name the first unreplaced parameter", e.getMessage().contains("firstUnknown")); + assertTrue("Exception message should name the second unreplaced parameter", e.getMessage().contains("secondUnknown")); + assertFalse("Exception message should not include the known, replaced parameter", e.getMessage().contains(KNOWN_PARAM)); + } + } } diff --git a/api/src/org/labkey/api/reports/report/r/RScriptEngine.java b/api/src/org/labkey/api/reports/report/r/RScriptEngine.java index 540e4bf0941..8a5d672cda3 100644 --- a/api/src/org/labkey/api/reports/report/r/RScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/r/RScriptEngine.java @@ -63,7 +63,7 @@ public ScriptEngineFactory getFactory() } @Override - protected FileLike prepareScriptFile(String script, ScriptContext context, List extensions) + protected FileLike prepareScriptFile(String script, ScriptContext context, List extensions) throws ScriptException { FileLike scriptFile; if (getKnitrFormat(context) != RReportDescriptor.KnitrFormat.None)