API key auth optional feature flag

.gitattributes

@@ -2742,7 +2742,6 @@ study/test/src/org/labkey/test/tests/study/AssayTest.java -text
 study/test/src/org/labkey/test/tests/study/CohortTest.java -text
 study/test/src/org/labkey/test/tests/study/ExtraKeyStudyTest.java -text
 study/test/src/org/labkey/test/tests/study/QuerySnapshotTest.java -text
-study/test/src/org/labkey/test/tests/study/SpecimenReplaceTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyCohortExportTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyContinuousTest.java -text
 study/test/src/org/labkey/test/tests/study/StudyDateBasedTest.java -text

api/src/org/labkey/api/security/AuthFilter.java

@@ -40,6 +40,7 @@
 import org.labkey.api.util.GUID;
 import org.labkey.api.util.HttpUtil;
 import org.labkey.api.util.HttpsUtil;
+import org.labkey.api.view.BadRequestException;
 import org.labkey.api.view.UnauthorizedException;
 import org.labkey.api.view.ViewServlet;
 
@@ -189,6 +190,11 @@ else if (!AppProps.getInstance().isDevMode())
             resp.sendError(HttpServletResponse.SC_BAD_REQUEST, uee.getMessage());
             return;
         }
+        catch (BadRequestException bre)
+        {
+            resp.sendError(bre.getStatus(), bre.getMessage());
+            return;
+        }
         catch (UnauthorizedException ue)
         {
             ExceptionUtil.handleException(req, resp, ue, ue.getMessage(), false);

api/src/org/labkey/api/security/SecurityManager.java

@@ -44,6 +44,8 @@
 import org.labkey.api.audit.AuditLogService;
 import org.labkey.api.audit.permissions.CanSeeAuditLogPermission;
 import org.labkey.api.audit.provider.GroupAuditProvider;
+import org.labkey.api.cache.CacheManager;
+import org.labkey.api.cache.Throttle;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerManager;
 import org.labkey.api.data.CoreSchema;
@@ -111,6 +113,7 @@
 import org.labkey.api.util.emailTemplate.UserOriginatedEmailTemplate;
 import org.labkey.api.util.logging.LogHelper;
 import org.labkey.api.view.ActionURL;
+import org.labkey.api.view.BadRequestException;
 import org.labkey.api.view.HasHttpRequest;
 import org.labkey.api.view.HttpView;
 import org.labkey.api.view.NotFoundException;
@@ -176,6 +179,8 @@ public class SecurityManager
     public static final String TRANSFORM_SESSION_ID = "LabKeyTransformSessionId";  // issue 19748
     /** 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 FEATURE_FLAG_ALLOW_APIKEY_PARAMETER = "AllowApiKeyParameter";
+    public static final String FEATURE_FLAG_ALLOW_APIKEY_PARAMETER_DESCRIPTION = "Allow authentication via 'apikey' URL parameter";
     public static final String API_KEY = "apikey";
 
     public static final String USER_ID_KEY = User.class.getName() + "$userId";
@@ -419,7 +424,13 @@ public void userAccountDisabled(User user)
         }
     }
 
-    private record Credentials(String username, String password) {}
+    private record Credentials(String username, String password, boolean shouldSetSessionCookie)
+    {
+        Credentials(String username, String password)
+        {
+            this(username, password, true);
+        }
+    }
 
     private static @Nullable Credentials getBasicCredentials(HttpServletRequest request)
     {
@@ -568,12 +579,16 @@ public record AuthenticationAttempt(User user, HttpServletRequest request) {}
                     String sessionId = PageFlowUtil.getCookieValue(request.getCookies(), JSESSIONID, null);
                     if (!session.getId().equals(sessionId))
                     {
-                        Cookie sessionCookie = new Cookie(JSESSIONID, session.getId());
-                        sessionCookie.setPath("/");
-                        sessionCookie.setHttpOnly(true);
-                        if (AppProps.getInstance().isSSLRequired() || request.isSecure())
-                            sessionCookie.setSecure(true);
-                        response.addCookie(sessionCookie);
+                        // A URL can be planted in a victim's browser, so a URL parameter key must not set the session cookie
+                        if (basicCredentials.shouldSetSessionCookie())
+                        {
+                            Cookie sessionCookie = new Cookie(JSESSIONID, session.getId());
+                            sessionCookie.setPath("/");
+                            sessionCookie.setHttpOnly(true);
+                            if (AppProps.getInstance().isSSLRequired() || request.isSecure())
+                                sessionCookie.setSecure(true);
+                            response.addCookie(sessionCookie);
+                        }
                         request = new SessionReplacingRequest(request, session);
                     }
                 }
@@ -670,8 +685,8 @@ public record AuthenticationAttempt(User user, HttpServletRequest request) {}
     /**
      * 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.
+     * the "apikey" GET parameter (supported since SSRS can't be made to use a header, but only if the optional feature
+     * flag is enabled). 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.
      */
@@ -680,6 +695,7 @@ public record AuthenticationAttempt(User user, HttpServletRequest request) {}
         // Passing via the "apikey" HTTP header is our preferred approach and used by most LabKey client API
         // implementations
         String apiKey = request.getHeader(API_KEY);
+        boolean shouldSetSessionCookie = true;
 
         if (null == apiKey)
         {
@@ -710,24 +726,54 @@ public record AuthenticationAttempt(User user, HttpServletRequest request) {}
             }
             else
             {
-                // 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.
+                // Continue to support "apikey" as a GET parameter only if the optional feature flag is enabled. This
+                // supports authentication through SSRS, which can't be made to use BasicAuth, pass cookies, or use HTTP
+                // headers.
+                Map<String, String> params;
                 try
                 {
-                    Map<String, String> params = PageFlowUtil.mapFromQueryString(request.getQueryString());
-                    apiKey = params.get(TRANSFORM_SESSION_ID);
+                    // Do not use request.getParameter() since that will consume the POST body, #32711.
+                    params = PageFlowUtil.mapFromQueryString(request.getQueryString());
                 }
                 catch (IllegalArgumentException e)
                 {
+                    // URLDecoder throws on malformed escapes; AuthFilter maps this to a 400
                     throw new UnsupportedEncodingException(e.getMessage());
                 }
+
+                String apiKeyParameter = params.get(API_KEY);
+
+                if (apiKeyParameter != null)
+                {
+                    if (AppProps.getInstance().isOptionalFeatureEnabled(FEATURE_FLAG_ALLOW_APIKEY_PARAMETER))
+                    {
+                        apiKey = apiKeyParameter;
+                        shouldSetSessionCookie = false;
+                    }
+                    else
+                    {
+                        API_KEY_PARAMETER_WARNING_THROTTLE.execute("Rejected \"" + API_KEY + "\" parameter; " +
+                            "enable the \"" + FEATURE_FLAG_ALLOW_APIKEY_PARAMETER_DESCRIPTION + "\" optional feature " +
+                            "flag or authenticate via a different approach.");
+                    }
+                }
+                else if (params.get(TRANSFORM_SESSION_ID) != null)
+                {
+                    String message = "Rejected \"" + TRANSFORM_SESSION_ID + "\" parameter because it's no longer " +
+                        "supported. Enable the \"" + FEATURE_FLAG_ALLOW_APIKEY_PARAMETER_DESCRIPTION + "\" optional " +
+                        "feature flag and use the \"" + API_KEY + "\" parameter instead.";
+                    API_KEY_PARAMETER_WARNING_THROTTLE.execute(message);
+                    throw new BadRequestException(message);
+                }
             }
         }
 
-        return null != apiKey ? new Credentials(API_KEY, apiKey) : null;
+        return null != apiKey ? new Credentials(API_KEY, apiKey, shouldSetSessionCookie) : null;
     }
 
+    // Unauthenticated callers can trigger these warnings on every request
+    private static final Throttle<String> API_KEY_PARAMETER_WARNING_THROTTLE = new Throttle<>("apikey parameter warnings", 10, CacheManager.HOUR, AUTH_LOG::warn);
+
     public static final int SECONDS_PER_DAY = 60*60*24;
 
     public static abstract class TransformSession implements Closeable

assay/src/org/labkey/assay/AssayModule.java

@@ -119,6 +119,7 @@
 
 import static org.labkey.api.assay.transform.DataTransformService.LEGACY_SESSION_COOKIE_NAME_REPLACEMENT;
 import static org.labkey.api.assay.transform.DataTransformService.LEGACY_SESSION_ID_REPLACEMENT;
+import static org.labkey.api.assay.transform.DataTransformService.R_SESSIONID_REPLACEMENT;
 
 public class AssayModule extends SpringModule
 {
@@ -184,6 +185,8 @@ protected void init()
 
         ParamReplacementSvc.get().registerDeprecated(LEGACY_SESSION_COOKIE_NAME_REPLACEMENT, ValidationException.SEVERITY.WARN, "Use '" + SecurityManager.API_KEY + "' instead");
         ParamReplacementSvc.get().registerDeprecated(LEGACY_SESSION_ID_REPLACEMENT, ValidationException.SEVERITY.WARN, "Use '" + SecurityManager.API_KEY + "' instead");
+        ParamReplacementSvc.get().registerDeprecated(R_SESSIONID_REPLACEMENT, ValidationException.SEVERITY.WARN, "Use '" + SecurityManager.API_KEY + "' instead");
+        ParamReplacementSvc.get().registerDeprecated(SecurityManager.TRANSFORM_SESSION_ID, ValidationException.SEVERITY.WARN, "Use '" + SecurityManager.API_KEY + "' instead");
 
         RoleManager.registerRole(new AssayDesignerRole());
 

core/src/org/labkey/core/CoreModule.java

@@ -373,6 +373,7 @@
 import java.util.stream.Stream;
 
 import static org.labkey.api.mcp.McpService.VECTOR_SCHEMA;
+import static org.labkey.api.security.SecurityManager.FEATURE_FLAG_ALLOW_APIKEY_PARAMETER_DESCRIPTION;
 import static org.labkey.api.settings.StashedStartupProperties.homeProjectFolderType;
 import static org.labkey.api.settings.StashedStartupProperties.homeProjectResetPermissions;
 import static org.labkey.api.settings.StashedStartupProperties.homeProjectWebparts;
@@ -543,6 +544,11 @@ public QuerySchema createSchema(DefaultSchema schema, Module module)
             "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().addFeatureFlag(new OptionalFeatureFlag(SecurityManager.FEATURE_FLAG_ALLOW_APIKEY_PARAMETER,
+            FEATURE_FLAG_ALLOW_APIKEY_PARAMETER_DESCRIPTION,
+            "Allows tools such as SSRS to authenticate by providing an API key via an 'apikey' parameter. Providing " +
+            "a credential via a URL parameter is not generally recommended, but in some cases this is the only option.",
+            false, false, FeatureType.Optional));
         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.",