Skip to content

API key auth optional feature flag #1909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-adam merged 9 commits into from
Oct 5, 2026
Merged

API key auth optional feature flag #1909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fb1086b
API key auth optional feature flag
labkey-adam Sep 29, 2026
163bd0c
Update comments
labkey-adam Sep 29, 2026
a6b0196
Ensure optional feature flag gets reset
labkey-adam Sep 29, 2026
f040914
Adjust error handling
labkey-adam Sep 29, 2026
207598e
Don't set the session cookie when using "apikey" parameter
labkey-adam Sep 29, 2026
101d71b
Enable optional feature flag by default
labkey-adam Sep 29, 2026
716b17f
Use constants
labkey-adam Sep 30, 2026
d39f0c2
Use a connection that's not tied to the primary test session to set/r…
labkey-adam Sep 30, 2026
1a17017
Merge branch 'develop' into fb_apikey_auth
labkey-martyp Oct 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions onprc_ehr/src/org/labkey/onprc_ehr/ONPRC_EHRModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@
import org.labkey.api.query.DetailsURL;
import org.labkey.api.query.QuerySchema;
import org.labkey.api.resource.Resource;
import org.labkey.api.security.SecurityManager;
import org.labkey.api.security.User;
import org.labkey.api.security.permissions.AdminPermission;
import org.labkey.api.security.roles.RoleManager;
import org.labkey.api.settings.AppProps;
import org.labkey.api.settings.OptionalFeatureService;
import org.labkey.api.util.URLHelper;
import org.labkey.api.util.UnexpectedException;
import org.labkey.api.view.ActionURL;
Expand Down Expand Up @@ -157,6 +160,11 @@ protected void init()
@Override
protected void doStartupAfterSpringConfig(ModuleContext moduleContext)
{
// SSRS reports authenticate via the apikey URL parameter. TODO: Remove this once ONPRC has upgraded to 26.11 or later.
OptionalFeatureService ofs = OptionalFeatureService.get();
if (!ofs.isFeatureEnabled(SecurityManager.FEATURE_FLAG_ALLOW_APIKEY_PARAMETER))
ofs.setFeatureEnabled(SecurityManager.FEATURE_FLAG_ALLOW_APIKEY_PARAMETER, true, User.getAdminServiceUser());

registerEHRResources();

NotificationService ns = NotificationService.get();
Expand Down
131 changes: 87 additions & 44 deletions onprc_ehr/test/src/org/labkey/test/tests/onprc_ehr/ONPRC_SsrsSessionKeyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.labkey.remoteapi.Connection;
import org.labkey.test.BaseWebDriverTest;
import org.labkey.test.ModulePropertyValue;
import org.labkey.test.TestTimeoutException;
import org.labkey.test.WebTestHelper;
import org.labkey.test.categories.ONPRC;
import org.labkey.test.util.OptionalFeatureHelper;
import org.labkey.test.util.PasswordUtil;
import org.labkey.test.util.SimpleHttpRequest;
import org.labkey.test.util.SimpleHttpResponse;
Expand Down Expand Up @@ -55,7 +57,7 @@
* onprc_ehr-getSessionId.api, which returns a session key (see ONPRC_EHRController.GetSessionIdAction).
* 2. Clicking a report builds a URL to the SSRS server carrying that key as the "SessionId" parameter.
* 3. SSRS, running on a separate host with no LabKey cookie, calls back to a selectRows URL, passing the
* key as the "LabKeyTransformSessionId" query parameter. SecurityManager.getApiKey() reads it from the
* key as the "apikey" query parameter. SecurityManager.getApiKey() reads it from the
* query string and SessionApiKeyManager resolves it back to the user's session.
* <p>
* The only thing we fake is SSRS: the SSRSServerURL module property points back at this LabKey instance (the
Expand All @@ -65,6 +67,10 @@
@Category({ONPRC.class})
public class ONPRC_SsrsSessionKeyTest extends BaseWebDriverTest
{
private final static String API_KEY_OPTIONAL_FEATURE_FLAG = "AllowApiKeyParameter";
private final static String API_KEY_PARAMETER_NAME = "apikey";
private final static String OLD_PARAMETER_NAME = "LabKeyTransformSessionId";

@Override
protected String getProjectName()
{
Expand Down Expand Up @@ -92,10 +98,11 @@ private void doSetup()
// Treat this LabKey instance as the fake SSRS target (mirrors AbstractGenericONPRC_EHRTest). preloadSession()
// does not actually need these, but setting them keeps the printable reports page behaving as in production.
setModuleProperties(Arrays.asList(
new ModulePropertyValue("ONPRC_EHR", "/" + getProjectName(), "SSRSServerURL", WebTestHelper.getBaseURL()),
new ModulePropertyValue("ONPRC_EHR", "/" + getProjectName(), "SSRSReportFolder", "DummySSRSFolder")
new ModulePropertyValue("ONPRC_EHR", "/" + getProjectName(), "SSRSServerURL", WebTestHelper.getBaseURL()),
new ModulePropertyValue("ONPRC_EHR", "/" + getProjectName(), "SSRSReportFolder", "DummySSRSFolder")
));
}

@Override
protected void checkQueries()
{
Expand All @@ -110,56 +117,81 @@ public void testSsrsSessionKeyAuthentication() throws IOException

// 2) Harvest the token exactly as the SSRS link would receive it
String sessionKey = waitFor(
() -> (String) executeScript("return (window.ONPRC && ONPRC.Utils) ? ONPRC.Utils.sessionId : null;"),
"ONPRC.Utils.preloadSession() never populated a session key", WAIT_FOR_JAVASCRIPT);
() -> (String) executeScript("return (window.ONPRC && ONPRC.Utils) ? ONPRC.Utils.sessionId : null;"),
"ONPRC.Utils.preloadSession() never populated a session key", WAIT_FOR_JAVASCRIPT);
assertNotNull("Session key was not preloaded", sessionKey);

// The key must be a session key, NOT the raw JSESSIONID -- that is the whole point of the change.
String jsessionId = getDriver().manage().getCookieNamed("JSESSIONID").getValue();
assertNotEquals("getSessionId returned the raw JSESSIONID instead of a session key", jsessionId, sessionKey);

String expectedEmail = PasswordUtil.getUsername();
Connection cn = WebTestHelper.getRemoteApiConnection(); // Not tied to test user session

// 3) Simulate the SSRS callback: cookieless, no Basic auth, ONLY the token on the URL.
// 3a) Identity check via whoami -- proves the callback authenticates as the right user.
JSONObject whoAmI = cookielessGetJson(WebTestHelper.buildURL("login", getProjectName(), "whoami",
Map.of("LabKeyTransformSessionId", sessionKey)));
assertEquals("Token-authenticated callback resolved to the wrong user", expectedEmail, whoAmI.getString("email"));

// 3b) Closest-to-real: the actual selectRows callback shape SSRS uses to fetch data. SSRS's XML data
// source extension requests the XML response format, so do the same and validate that the payload is
// well-formed XML containing the expected data row (the current user, filtered by email).
SimpleHttpResponse selectRows = cookielessGet(WebTestHelper.buildURL("query", getProjectName(), "selectRows",
try
{
// Attempt authentication with the optional feature flag off
OptionalFeatureHelper.disableOptionalFeature(cn, API_KEY_OPTIONAL_FEATURE_FLAG);
JSONObject featureOff = cookielessGetJson(WebTestHelper.buildURL("login", getProjectName(), "whoami",
Map.of(API_KEY_PARAMETER_NAME, sessionKey)));
assertEquals("With optional feature off, " + API_KEY_PARAMETER_NAME + " parameter should have been ignored, resulting in guest", "guest", featureOff.getString("email"));

// Turn on the optional feature flag for the rest of the test
OptionalFeatureHelper.enableOptionalFeature(cn, API_KEY_OPTIONAL_FEATURE_FLAG);

// Attempt authentication using the old, unsupported parameter name
SimpleHttpResponse oldParameter = cookielessGet(WebTestHelper.buildURL("login", getProjectName(), "whoami",
Map.of(OLD_PARAMETER_NAME, sessionKey)));
assertEquals(OLD_PARAMETER_NAME + " parameter should have been rejected", 400, oldParameter.getResponseCode());

// 3) Simulate the SSRS callback: cookieless, no Basic auth, ONLY the token on the URL.
// 3a) Identity check via whoami -- proves the callback authenticates as the right user.
SimpleHttpResponse whoAmIResponse = cookielessGet(WebTestHelper.buildURL("login", getProjectName(), "whoami",
Map.of(API_KEY_PARAMETER_NAME, sessionKey)));
JSONObject whoAmI = new JSONObject(whoAmIResponse.getResponseBody());
assertEquals("Token-authenticated callback resolved to the wrong user", expectedEmail, whoAmI.getString("email"));
assertNoSessionCookie(whoAmIResponse);

// 3b) Closest-to-real: the actual selectRows callback shape SSRS uses to fetch data. SSRS's XML data
// source extension requests the XML response format, so do the same and validate that the payload is
// well-formed XML containing the expected data row (the current user, filtered by email).
SimpleHttpResponse selectRows = cookielessGet(WebTestHelper.buildURL("query", getProjectName(), "selectRows",
Map.of("schemaName", "core", "query.queryName", "Users", "query.columns", "Email",
"query.Email~eq", expectedEmail, "respFormat", "xml", "LabKeyTransformSessionId", sessionKey)));
assertEquals("selectRows callback with a valid token should succeed", 200, selectRows.getResponseCode());

Document doc = parseXml(selectRows.getResponseBody());
Element root = doc.getDocumentElement();
assertEquals("Unexpected root element in selectRows XML response", "response", root.getTagName());
Element rowsElement = (Element) root.getElementsByTagName("rows").item(0);
assertNotNull("selectRows XML response is missing the <rows> element", rowsElement);
NodeList rows = rowsElement.getElementsByTagName("element");
assertTrue("selectRows XML response should contain at least one data row", rows.getLength() >= 1);
Node email = ((Element) rows.item(0)).getElementsByTagName("Email").item(0);
assertNotNull("Data row in selectRows XML response is missing the Email column", email);
assertEquals("Data row in selectRows XML response should be for the current user", expectedEmail, email.getTextContent());

// 4) Negative controls -- prove it is the token doing the work.
// 4a) No token -> guest (empty email)
JSONObject noToken = cookielessGetJson(WebTestHelper.buildURL("login", getProjectName(), "whoami"));
assertEquals("A cookieless callback with no token should be guest", "guest", noToken.getString("email"));

// 4b) Bogus token -> guest
JSONObject bogus = cookielessGetJson(WebTestHelper.buildURL("login", getProjectName(), "whoami",
Map.of("LabKeyTransformSessionId", "not-a-real-session-key")));
assertFalse("A cookieless callback with a bogus token should be guest", bogus.getBoolean("success"));

// 5) Lifecycle: after the user logs out, the session key must stop working (auto-invalidated with the session).
signOut();
JSONObject afterLogout = cookielessGetJson(WebTestHelper.buildURL("login", getProjectName(), "whoami",
Map.of("LabKeyTransformSessionId", sessionKey)));
assertFalse("A cookieless callback with a bogus token should be guest", afterLogout.getBoolean("success"));
"query.Email~eq", expectedEmail, "respFormat", "xml", API_KEY_PARAMETER_NAME, sessionKey)));
assertEquals("selectRows callback with a valid token should succeed", 200, selectRows.getResponseCode());
assertNoSessionCookie(selectRows);

Document doc = parseXml(selectRows.getResponseBody());
Element root = doc.getDocumentElement();
assertEquals("Unexpected root element in selectRows XML response", "response", root.getTagName());
Element rowsElement = (Element) root.getElementsByTagName("rows").item(0);
assertNotNull("selectRows XML response is missing the <rows> element", rowsElement);
NodeList rows = rowsElement.getElementsByTagName("element");
assertTrue("selectRows XML response should contain at least one data row", rows.getLength() >= 1);
Node email = ((Element) rows.item(0)).getElementsByTagName("Email").item(0);
assertNotNull("Data row in selectRows XML response is missing the Email column", email);
assertEquals("Data row in selectRows XML response should be for the current user", expectedEmail, email.getTextContent());

// 4) Negative controls -- prove it is the token doing the work.
// 4a) No token -> guest (empty email)
JSONObject noToken = cookielessGetJson(WebTestHelper.buildURL("login", getProjectName(), "whoami"));
assertEquals("A cookieless callback with no token should be guest", "guest", noToken.getString("email"));

// 4b) Bogus token -> guest
JSONObject bogus = cookielessGetJson(WebTestHelper.buildURL("login", getProjectName(), "whoami",
Map.of(API_KEY_PARAMETER_NAME, "not-a-real-session-key")));
assertFalse("A cookieless callback with a bogus token should be guest", bogus.getBoolean("success"));

// 5) Lifecycle: after the user logs out, the session key must stop working (auto-invalidated with the session).
signOut();
JSONObject afterLogout = cookielessGetJson(WebTestHelper.buildURL("login", getProjectName(), "whoami",
Map.of(API_KEY_PARAMETER_NAME, sessionKey)));
assertFalse("A cookieless callback with a bogus token should be guest", afterLogout.getBoolean("success"));
}
finally
{
OptionalFeatureHelper.resetOptionalFeature(cn, API_KEY_OPTIONAL_FEATURE_FLAG);
Comment thread
labkey-adam marked this conversation as resolved.
}
}

/**
Expand All @@ -178,6 +210,17 @@ private JSONObject cookielessGetJson(String url) throws IOException
return new JSONObject(cookielessGet(url).getResponseBody());
}

// An apikey URL parameter must not plant a session cookie, since the URL could have come from an attacker
private void assertNoSessionCookie(SimpleHttpResponse response)
{
List<String> sessionCookies = response.getResponseHeaderFields().entrySet().stream()
.filter(e -> "Set-Cookie".equalsIgnoreCase(e.getKey()))
.flatMap(e -> e.getValue().stream())
.filter(cookie -> cookie.startsWith("JSESSIONID="))
.toList();
assertTrue(API_KEY_PARAMETER_NAME + " URL parameter should not set a JSESSIONID cookie: " + sessionCookies, sessionCookies.isEmpty());
}

/**
* Parse a response body, failing the test if it is not well-formed XML.
*/
Expand Down
Loading