From 511e04e42a73cb78df4b658e330c00c558f6b6b0 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 16 May 2026 13:21:23 -0700 Subject: [PATCH 01/13] Support renew=true parameter for our CAS IdP --- api/src/org/labkey/api/security/AuthFilter.java | 17 +++++++++-------- .../api/security/AuthenticatedRequest.java | 13 ++++++++----- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthFilter.java b/api/src/org/labkey/api/security/AuthFilter.java index aac2863e5c5..966a7357d1b 100644 --- a/api/src/org/labkey/api/security/AuthFilter.java +++ b/api/src/org/labkey/api/security/AuthFilter.java @@ -199,14 +199,9 @@ else if (!AppProps.getInstance().isDevMode()) } if (null == user) - { - if (AppProps.getInstance().isOptionalFeatureEnabled(AppProps.EXPERIMENTAL_NO_GUESTS)) - user = User.nobody; - else - user = User.guest; - } + user = getGuestUser(); else - UserManager.updateRecentUser(user.isImpersonated() ? user.getImpersonatingUser() : user); // TODO: Sanity check this with Matt... treat impersonating admin as active, not impersonated user + UserManager.updateRecentUser(user.isImpersonated() ? user.getImpersonatingUser() : user); req = AuthenticatedRequest.create(req, user); @@ -262,7 +257,13 @@ private void addRandomHeader(HttpServletRequest req, HttpServletResponse resp) resp.addHeader("X-LK-NONCE", sb.toString()); } - + public static User getGuestUser() + { + if (AppProps.getInstance().isOptionalFeatureEnabled(AppProps.EXPERIMENTAL_NO_GUESTS)) + return User.nobody; + else + return User.guest; + } private boolean clearRequestAttributes(HttpServletRequest request) { diff --git a/api/src/org/labkey/api/security/AuthenticatedRequest.java b/api/src/org/labkey/api/security/AuthenticatedRequest.java index 690e1f5b0ab..26a91d18f72 100644 --- a/api/src/org/labkey/api/security/AuthenticatedRequest.java +++ b/api/src/org/labkey/api/security/AuthenticatedRequest.java @@ -55,15 +55,12 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; -/** - * User: matthewb - * Date: Feb 5, 2009 - */ public class AuthenticatedRequest extends HttpServletRequestWrapper implements AutoCloseable { private static final Logger _log = LogManager.getLogger(AuthenticatedRequest.class); private final User _user; + private boolean _loggedIn; private HttpSession _session = null; @@ -76,11 +73,17 @@ public static AuthenticatedRequest create(@NotNull HttpServletRequest request, @ private AuthenticatedRequest(@NotNull HttpServletRequest request, @NotNull User user) { - super(request instanceof AuthenticatedRequest ? (HttpServletRequest)((AuthenticatedRequest)request).getRequest() : request); + super(request instanceof AuthenticatedRequest authRequest ? authRequest.getRequest() : request); _user = user; _loggedIn = !_user.isGuest(); } + @Override + public HttpServletRequest getRequest() + { + return (HttpServletRequest)super.getRequest(); + } + @Override public void close() { From 84dddf44861c01078fbe027f53306fe1b15ff66f Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 18 May 2026 11:17:33 -0700 Subject: [PATCH 02/13] Skip the "timestamps shouldn't match" checks on SQL Server --- .../src/org/labkey/assay/AssayIntegrationTestCase.jsp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp b/assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp index cb7e846a345..c1df4c52dd6 100644 --- a/assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp +++ b/assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp @@ -586,10 +586,13 @@ // verify result created matches run's created in query table, but result modified now differs from run's created Map modifiedResults = new TableSelector(resultsTable, selectColumns, new SimpleFilter(runFieldKey, runRowId), null).getMap(); assertEquals("modifiedResults Created didn't match runOriginalCreated", modifiedResults.get("Created"), runOriginalCreated); - assertNotEquals("modifiedResults Created shouldn't match modifiedResult Modified", modifiedResults.get("Created"), modifiedResults.get("Modified")); - assertNotEquals("modifiedResults Modified shouldn't match runOriginalCreated", modifiedResults.get("Modified"), runOriginalCreated); - assertNotEquals("modifiedResults Modified shouldn't match runOriginalModified", modifiedResults.get("Modified"), runOriginalModified); - assertNotEquals("modifiedResults Modified shouldn't match modifiedRunResults Modified", modifiedResults.get("Modified"), modifiedRunResults.get("Modified")); + if (schema.getDbSchema().getSqlDialect().isPostgreSQL()) + { + assertNotEquals("modifiedResults Created shouldn't match modifiedResult Modified", modifiedResults.get("Created"), modifiedResults.get("Modified")); + assertNotEquals("modifiedResults Modified shouldn't match runOriginalCreated", modifiedResults.get("Modified"), runOriginalCreated); + assertNotEquals("modifiedResults Modified shouldn't match runOriginalModified", modifiedResults.get("Modified"), runOriginalModified); + assertNotEquals("modifiedResults Modified shouldn't match modifiedRunResults Modified", modifiedResults.get("Modified"), modifiedRunResults.get("Modified")); + } // verify modified in provisioned result table no longer null after result edit dbResult = getRealResult(resultsTable.getSchema(), realResultsTable.getName(), resultRowId); From 67118f5d1733edc8ac189b8e8b80595eccc6ad16 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 18 May 2026 12:02:52 -0700 Subject: [PATCH 03/13] Increase sleepy time to work around SQL Server intermittent failures --- .../org/labkey/assay/AssayIntegrationTestCase.jsp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp b/assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp index c1df4c52dd6..491d652a502 100644 --- a/assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp +++ b/assay/src/org/labkey/assay/AssayIntegrationTestCase.jsp @@ -580,19 +580,16 @@ updated.put("ResultProp", 200); updated.put("RowId", resultRowId); errors = new BatchValidationException(); - Thread.sleep(5); // SQL Server timestamps aren't granular enough to guarantee different modified time + Thread.sleep(schema.getDbSchema().getSqlDialect().isSqlServer() ? 100 : 5); // SQL Server timestamps aren't granular enough to guarantee different modified time resultsQUS.updateRows(user, c, Collections.singletonList(updated), null, errors, null, null); // verify result created matches run's created in query table, but result modified now differs from run's created Map modifiedResults = new TableSelector(resultsTable, selectColumns, new SimpleFilter(runFieldKey, runRowId), null).getMap(); assertEquals("modifiedResults Created didn't match runOriginalCreated", modifiedResults.get("Created"), runOriginalCreated); - if (schema.getDbSchema().getSqlDialect().isPostgreSQL()) - { - assertNotEquals("modifiedResults Created shouldn't match modifiedResult Modified", modifiedResults.get("Created"), modifiedResults.get("Modified")); - assertNotEquals("modifiedResults Modified shouldn't match runOriginalCreated", modifiedResults.get("Modified"), runOriginalCreated); - assertNotEquals("modifiedResults Modified shouldn't match runOriginalModified", modifiedResults.get("Modified"), runOriginalModified); - assertNotEquals("modifiedResults Modified shouldn't match modifiedRunResults Modified", modifiedResults.get("Modified"), modifiedRunResults.get("Modified")); - } + assertNotEquals("modifiedResults Created shouldn't match modifiedResult Modified", modifiedResults.get("Created"), modifiedResults.get("Modified")); + assertNotEquals("modifiedResults Modified shouldn't match runOriginalCreated", modifiedResults.get("Modified"), runOriginalCreated); + assertNotEquals("modifiedResults Modified shouldn't match runOriginalModified", modifiedResults.get("Modified"), runOriginalModified); + assertNotEquals("modifiedResults Modified shouldn't match modifiedRunResults Modified", modifiedResults.get("Modified"), modifiedRunResults.get("Modified")); // verify modified in provisioned result table no longer null after result edit dbResult = getRealResult(resultsTable.getSchema(), realResultsTable.getName(), resultRowId); From d6f06cb80a2ee4daebebf036ecc5bbef4cd880d0 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 23 May 2026 19:44:22 -0700 Subject: [PATCH 04/13] Rework re-auth --- .../api/action/PermissionCheckableAction.java | 7 ++ .../api/security/AuthenticationManager.java | 7 ++ .../org/labkey/api/security/LoginUrls.java | 1 + .../org/labkey/api/util/ExceptionUtil.java | 6 +- .../labkey/api/view/ForceReauthException.java | 13 ++++ .../labkey/api/view/RedirectException.java | 4 +- .../api/view/UnauthorizedException.java | 29 +++++++- .../filters/ContentSecurityPolicyFilter.java | 1 - .../labkey/core/login/LoginController.java | 74 ++++++++++++------- core/webapp/login.js | 3 +- 10 files changed, 110 insertions(+), 35 deletions(-) create mode 100644 api/src/org/labkey/api/view/ForceReauthException.java diff --git a/api/src/org/labkey/api/action/PermissionCheckableAction.java b/api/src/org/labkey/api/action/PermissionCheckableAction.java index ad538934317..e14d93a6b26 100644 --- a/api/src/org/labkey/api/action/PermissionCheckableAction.java +++ b/api/src/org/labkey/api/action/PermissionCheckableAction.java @@ -42,6 +42,7 @@ import org.labkey.api.util.HttpUtil; import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.BadRequestException; +import org.labkey.api.view.ForceReauthException; import org.labkey.api.view.NotFoundException; import org.labkey.api.view.RedirectException; import org.labkey.api.view.UnauthorizedException; @@ -154,6 +155,12 @@ private void _checkActionPermissions(Set contextualRoles) throws Unauthori if (LOG.isDebugEnabled()) LOG.debug("{}: checking permissions for user {}", actionClass.getName(), user == null ? "" : user.getName() + " (impersonated=" + user.isImpersonated() + ")"); + // If _forceReauth is requested, unconditionally redirect to the login page + if (Boolean.parseBoolean((String)getViewContext().get(ForceReauthException.FORCE_REAUTH_NAME))) + { + throw new ForceReauthException(); + } + if (!actionClass.isAnnotationPresent(IgnoresForbiddenProjectCheck.class)) c.throwIfForbiddenProject(user); diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index d8054bb6cc5..888a044e7f0 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -1759,6 +1759,13 @@ public static URLHelper getWelcomeURL() return new URLHelper(true); } + public static final String REAUTH_TOKEN_NAME = "reauthToken"; + + public static boolean reauthTokenMatches(HttpServletRequest request, String token) + { + HttpSession session = request.getSession(true); + return token != null && token.equals(session.getAttribute(REAUTH_TOKEN_NAME)); + } // test() method should return true if the authentication is still valid public interface AuthenticationValidator extends Predicate diff --git a/api/src/org/labkey/api/security/LoginUrls.java b/api/src/org/labkey/api/security/LoginUrls.java index 3a7652fb9c0..aa604fae023 100644 --- a/api/src/org/labkey/api/security/LoginUrls.java +++ b/api/src/org/labkey/api/security/LoginUrls.java @@ -35,6 +35,7 @@ public interface LoginUrls extends UrlProvider ActionURL getLoginURL(URLHelper returnUrl); ActionURL getRegisterURL(Container c, @Nullable URLHelper returnUrl); ActionURL getLoginURL(Container c, @Nullable URLHelper returnUrl); + ActionURL getForceReauthURL(Container c, @Nullable URLHelper returnUrl); ActionURL getLogoutURL(Container c); ActionURL getLogoutURL(Container c, URLHelper returnUrl); ActionURL getStopImpersonatingURL(Container c, @Nullable URLHelper returnUrl); diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index 0434f794d78..b56c2f77c50 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -939,7 +939,7 @@ else if (ex instanceof UnauthorizedException uae) if (isGET) { // If user has not logged in or agreed to terms, not really unauthorized yet... - if (!isCSRFViolation && isGuest && type == UnauthorizedException.Type.redirectToLogin && !overrideBasicAuth && HttpView.hasCurrentView()) + if (!isCSRFViolation && type.isRedirect(isGuest) && !overrideBasicAuth && HttpView.hasCurrentView()) { // Issue 43307: If this is a locked project then just show the login page in the root. Register, // forgot my password, profile update, password reset, etc. aren't going to work right in a locked @@ -949,7 +949,9 @@ else if (ex instanceof UnauthorizedException uae) c = ContainerManager.getRoot(); // Issue 43387 - Retain original container info on login redirect URL, even if it resolved to something else - ActionURL loginURL = PageFlowUtil.urlProvider(LoginUrls.class).getLoginURL(c, HttpView.getContextURLHelper()); + LoginUrls urls = PageFlowUtil.urlProvider(LoginUrls.class); + URLHelper returnURL = HttpView.getContextURLHelper(); + ActionURL loginURL = (type == UnauthorizedException.Type.redirectToLogin ? urls.getLoginURL(c, returnURL) : urls.getForceReauthURL(c, returnURL)); Path originalContainerPath = (Path)request.getAttribute(ViewServlet.ORIGINAL_URL_CONTAINER_PATH); if (originalContainerPath != null) { diff --git a/api/src/org/labkey/api/view/ForceReauthException.java b/api/src/org/labkey/api/view/ForceReauthException.java new file mode 100644 index 00000000000..8d4f20b2e01 --- /dev/null +++ b/api/src/org/labkey/api/view/ForceReauthException.java @@ -0,0 +1,13 @@ +package org.labkey.api.view; + +// Use a subclass to prevent permission-checking code paths from setting the wrong type +public class ForceReauthException extends UnauthorizedException +{ + public static final String FORCE_REAUTH_NAME = "_forceReauth"; + + @Override + public Type getType() + { + return Type.forceReauth; + } +} diff --git a/api/src/org/labkey/api/view/RedirectException.java b/api/src/org/labkey/api/view/RedirectException.java index 6d832af899a..58519493062 100644 --- a/api/src/org/labkey/api/view/RedirectException.java +++ b/api/src/org/labkey/api/view/RedirectException.java @@ -23,8 +23,8 @@ /** * When thrown in the context of an HTTP request, sends the client a *temporary* redirect in the HTTP response. Not * treated as a loggable error. See {@link PermanentRedirectException} if a permanent redirect is desired. - * Note: This always redirects to the local server. If an external redirect is needed (this is rare), use - * {@link ExternalRedirectException} or (even rarer) {@link UnsafeExternalRedirectException}. + * Note: Instances of this specific class always redirect to the local server. If an external redirect is needed (this + * is rare), use {@link ExternalRedirectException} or (even rarer) {@link UnsafeExternalRedirectException}. */ public class RedirectException extends RuntimeException implements SkipMothershipLogging { diff --git a/api/src/org/labkey/api/view/UnauthorizedException.java b/api/src/org/labkey/api/view/UnauthorizedException.java index 564f0264ec1..d44e15d89cc 100644 --- a/api/src/org/labkey/api/view/UnauthorizedException.java +++ b/api/src/org/labkey/api/view/UnauthorizedException.java @@ -30,12 +30,33 @@ public class UnauthorizedException extends HttpStatusException /** Options for how the client should be informed of not being allowed to see a resource */ public enum Type { - /** Redirect the browser to a different URL to render a login form */ - redirectToLogin, + /** Always redirect the browser to the login page to force re-authentication */ + forceReauth() + { + @Override + public boolean isRedirect(boolean isGuest) + { + return true; + } + }, + /** If the user is guest, redirect the browser to render the login page */ + redirectToLogin() + { + @Override + public boolean isRedirect(boolean isGuest) + { + return isGuest; + } + }, /** Send a 401, but signal that the server would accept HTTP BasicAuth credentials */ - sendBasicAuth, + sendBasicAuth(), /** Send a 401 and don't solicit BasicAuth credentials */ - sendUnauthorized + sendUnauthorized(); + + public boolean isRedirect(boolean isGuest) + { + return false; + } } Type _type = Type.redirectToLogin; diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index dc077525b6b..743f0594e14 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -75,7 +75,6 @@ public class ContentSecurityPolicyFilter implements Filter private String _stashedTemplate = null; // We can't set this statically because the class is referenced before URLProviders are available - @SuppressWarnings("DataFlowIssue") private final String _reportingEndpointsHeaderValue = "csp-report=\"" + PageFlowUtil.urlProvider(AdminUrls.class).getCspReportToURL().getLocalURIString() + "\""; // Initialized on first request and reset when allowed sources change. Don't reference this directly; always use diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index c568c93240d..960a41d7713 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -92,6 +92,7 @@ import org.labkey.api.settings.WriteableLookAndFeelProperties; import org.labkey.api.util.CSRFUtil; import org.labkey.api.util.ConfigurationException; +import org.labkey.api.util.GUID; import org.labkey.api.util.HelpTopic; import org.labkey.api.util.HtmlString; import org.labkey.api.util.MailHelper; @@ -104,6 +105,7 @@ import org.labkey.api.view.ActionURL; import org.labkey.api.view.BadRequestException; import org.labkey.api.view.ExternalRedirectException; +import org.labkey.api.view.ForceReauthException; import org.labkey.api.view.HtmlView; import org.labkey.api.view.HttpView; import org.labkey.api.view.JspView; @@ -139,6 +141,7 @@ import static org.labkey.api.security.AuthenticationManager.LOGIN_ATTEMPT_LIMIT_KEY; import static org.labkey.api.security.AuthenticationManager.LOGIN_ATTEMPT_PERIOD_KEY; import static org.labkey.api.security.AuthenticationManager.LOGIN_ATTEMPT_RESET_TIME_KEY; +import static org.labkey.api.security.AuthenticationManager.REAUTH_TOKEN_NAME; import static org.labkey.api.security.AuthenticationManager.SELF_REGISTRATION_KEY; import static org.labkey.api.security.AuthenticationManager.SELF_SERVICE_EMAIL_CHANGES_KEY; @@ -244,6 +247,16 @@ public ActionURL getLoginURL(Container c, @Nullable URLHelper returnUrl) return url; } + @Override + public ActionURL getForceReauthURL(Container c, @Nullable URLHelper returnUrl) + { + if (returnUrl != null) + returnUrl.deleteParameter(ForceReauthException.FORCE_REAUTH_NAME); + + return getLoginURL(c, returnUrl) + .addParameter("forceReauth", true); + } + @Override public ActionURL getRegisterURL(Container c, @Nullable URLHelper returnUrl) { @@ -387,7 +400,7 @@ public class RegisterAction extends SimpleViewAction @Override public ModelAndView getView(RegisterForm form, BindException errors) { - ModelAndView redirectView = redirectIfLoggedIn(form); + ModelAndView redirectView = redirectIfLoggedIn(form, false); if (redirectView != null) return redirectView; if (!AuthenticationManager.isRegistrationEnabled()) @@ -581,19 +594,20 @@ public void setProvider(String provider) * @return a view that will redirect the user if they're already logged in, or null if they're not logged in already */ @Nullable - private ModelAndView redirectIfLoggedIn(AbstractLoginForm form) + private ModelAndView redirectIfLoggedIn(AbstractLoginForm form, boolean forceLogin) { - if (!getUser().isGuest()) + if (getUser().isGuest() || forceLogin) { - URLHelper returnUrl = form.getReturnUrlHelper(); + return null; + } - // Create LoginReturnProperties if we have a returnUrl or skipProfile param - LoginReturnProperties properties = null != returnUrl || form.getSkipProfile() - ? new LoginReturnProperties(returnUrl, form.getUrlhash(), form.getSkipProfile()) : null; + URLHelper returnUrl = form.getReturnUrlHelper(); - throw new ExternalRedirectException(AuthenticationManager.getAfterLoginURL(getContainer(), properties, getUser())); - } - return null; + // Create LoginReturnProperties if we have a returnUrl or skipProfile param + LoginReturnProperties properties = null != returnUrl || form.getSkipProfile() + ? new LoginReturnProperties(returnUrl, form.getUrlhash(), form.getSkipProfile()) : null; + + throw new ExternalRedirectException(AuthenticationManager.getAfterLoginURL(getContainer(), properties, getUser())); } @RequiresNoPermission @@ -609,7 +623,7 @@ public ModelAndView getView(LoginForm form, BindException errors) var canonicalUrl = PageFlowUtil.urlProvider(LoginUrls.class).getLoginURL(ContainerManager.getRoot(), null); getPageConfig().setCanonicalLink(canonicalUrl.getURIString()); - ModelAndView redirectView = redirectIfLoggedIn(form); + ModelAndView redirectView = redirectIfLoggedIn(form, form.isForceReauth()); if (redirectView != null) return redirectView; HttpServletRequest request = getViewContext().getRequest(); @@ -691,6 +705,13 @@ else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE) response.put("approvedTermsOfUse", true); } + if (form.isForceReauth()) + { + String reauthToken = GUID.makeHash(); + redirectUrl.addParameter(REAUTH_TOKEN_NAME, reauthToken); + request.getSession().setAttribute(REAUTH_TOKEN_NAME, reauthToken); + } + // Use the full hostname in the URL if we have one, otherwise just go with a local URI String redirectString = redirectUrl.getHost() != null && redirectUrl.getScheme() != null ? redirectUrl.getURIString() : redirectUrl.toString(); @@ -1364,23 +1385,20 @@ public void setApprovedTermsOfUse(boolean approvedTermsOfUse) public static class LoginForm extends AgreeToTermsForm { - private boolean remember; private String email; private String password; private String provider; + private boolean forceReauth = false; - public void setProvider(String provider) - { - this.provider = provider; - } - public void setEmail(String email) + public String getProvider() { - this.email = email; + return this.provider; } - public String getProvider() + @SuppressWarnings({"UnusedDeclaration"}) + public void setProvider(String provider) { - return this.provider; + this.provider = provider; } public String getEmail() @@ -1388,6 +1406,12 @@ public String getEmail() return this.email; } + @SuppressWarnings({"UnusedDeclaration"}) + public void setEmail(String email) + { + this.email = email; + } + public String getPassword() { return password; @@ -1399,15 +1423,15 @@ public void setPassword(String password) this.password = password; } - public boolean isRemember() + public boolean isForceReauth() { - return this.remember; + return forceReauth; } - @SuppressWarnings({"UnusedDeclaration"}) - public void setRemember(boolean remember) + @SuppressWarnings("unused") + public void setForceReauth(boolean forceReauth) { - this.remember = remember; + this.forceReauth = forceReauth; } } diff --git a/core/webapp/login.js b/core/webapp/login.js index e4f4832fe51..642f7c0be50 100644 --- a/core/webapp/login.js +++ b/core/webapp/login.js @@ -44,7 +44,8 @@ termsOfUseType: document.getElementById('termsOfUseType').value, returnUrl: returnUrlElement && returnUrlElement.value ? returnUrlElement.value : LABKEY.ActionURL.getParameter("returnUrl"), skipProfile: LABKEY.ActionURL.getParameter("skipProfile") || 0, - urlhash: document.getElementById('urlhash').value + urlhash: document.getElementById('urlhash').value, + forceReauth: LABKEY.ActionURL.getParameter("forceReauth") || false }, success: LABKEY.Utils.getCallbackWrapper(function(response) { setSubmitting(false, [{msg: ''}]); From aa7718bada8f34f5595c3c56c90472b0f3ea549b Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 23 May 2026 20:11:14 -0700 Subject: [PATCH 05/13] Remove re-auth token after checking match. Stop posting remember me parameter that's client-side only. --- .../labkey/api/security/AuthenticationManager.java | 14 ++++++++++++-- core/webapp/login.js | 1 - 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 888a044e7f0..46e419c6bf3 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -1763,8 +1763,18 @@ public static URLHelper getWelcomeURL() public static boolean reauthTokenMatches(HttpServletRequest request, String token) { - HttpSession session = request.getSession(true); - return token != null && token.equals(session.getAttribute(REAUTH_TOKEN_NAME)); + if (token == null) + return false; + + HttpSession session = request.getSession(false); + + if (session == null) + return false; + + boolean matches = token.equals(session.getAttribute(REAUTH_TOKEN_NAME)); + session.removeAttribute(REAUTH_TOKEN_NAME); + + return matches; } // test() method should return true if the authentication is still valid diff --git a/core/webapp/login.js b/core/webapp/login.js index 642f7c0be50..e9c4bcc9199 100644 --- a/core/webapp/login.js +++ b/core/webapp/login.js @@ -37,7 +37,6 @@ url: LABKEY.ActionURL.buildURL('login', 'loginApi.api', this.containerPath), method: 'POST', params: { - remember: document.getElementById('remember').value, email: document.getElementById('email').value, password: document.getElementById('password').value, approvedTermsOfUse: document.getElementById('approvedTermsOfUse').checked, From 53a84e1f7bf28a13ca91cec8c23a0e8a0729907d Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 23 May 2026 20:19:54 -0700 Subject: [PATCH 06/13] remove conditionally --- api/src/org/labkey/api/security/AuthenticationManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 46e419c6bf3..f89ff330608 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -1772,7 +1772,9 @@ public static boolean reauthTokenMatches(HttpServletRequest request, String toke return false; boolean matches = token.equals(session.getAttribute(REAUTH_TOKEN_NAME)); - session.removeAttribute(REAUTH_TOKEN_NAME); + + if (matches) + session.removeAttribute(REAUTH_TOKEN_NAME); return matches; } From 3079a6d1f8ca03181f2526a517ec4104c7c1ee62 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 24 May 2026 09:43:38 -0700 Subject: [PATCH 07/13] Simplify with a direct redirect to login action --- .../api/action/PermissionCheckableAction.java | 7 ---- .../api/security/AuthenticationManager.java | 20 ++++++++---- .../org/labkey/api/security/LoginUrls.java | 4 ++- .../org/labkey/api/util/ExceptionUtil.java | 6 ++-- .../labkey/api/view/ForceReauthException.java | 13 -------- .../api/view/UnauthorizedException.java | 32 +++---------------- .../labkey/core/login/LoginController.java | 18 +++++++---- 7 files changed, 35 insertions(+), 65 deletions(-) delete mode 100644 api/src/org/labkey/api/view/ForceReauthException.java diff --git a/api/src/org/labkey/api/action/PermissionCheckableAction.java b/api/src/org/labkey/api/action/PermissionCheckableAction.java index e14d93a6b26..ad538934317 100644 --- a/api/src/org/labkey/api/action/PermissionCheckableAction.java +++ b/api/src/org/labkey/api/action/PermissionCheckableAction.java @@ -42,7 +42,6 @@ import org.labkey.api.util.HttpUtil; import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.BadRequestException; -import org.labkey.api.view.ForceReauthException; import org.labkey.api.view.NotFoundException; import org.labkey.api.view.RedirectException; import org.labkey.api.view.UnauthorizedException; @@ -155,12 +154,6 @@ private void _checkActionPermissions(Set contextualRoles) throws Unauthori if (LOG.isDebugEnabled()) LOG.debug("{}: checking permissions for user {}", actionClass.getName(), user == null ? "" : user.getName() + " (impersonated=" + user.isImpersonated() + ")"); - // If _forceReauth is requested, unconditionally redirect to the login page - if (Boolean.parseBoolean((String)getViewContext().get(ForceReauthException.FORCE_REAUTH_NAME))) - { - throw new ForceReauthException(); - } - if (!actionClass.isAnnotationPresent(IgnoresForbiddenProjectCheck.class)) c.throwIfForbiddenProject(user); diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index f89ff330608..d8544297aab 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -1619,6 +1619,11 @@ public URLHelper getRedirectURL() public static @NotNull AuthenticationResult handleAuthentication(HttpServletRequest request, Container c) + { + return handleAuthentication(request, c, true); + } + + public static @NotNull AuthenticationResult handleAuthentication(HttpServletRequest request, Container c, boolean setSession) { HttpSession session = request.getSession(true); PrimaryAuthenticationResult primaryAuthResult = AuthenticationManager.getPrimaryAuthenticationResult(session); @@ -1686,13 +1691,16 @@ public URLHelper getRedirectURL() LoginReturnProperties properties = getLoginReturnProperties(request); URLHelper url = getAfterLoginURL(c, properties, primaryAuthUser); - // Prep the new session and set the user & authentication-related attributes - session = SecurityManager.setAuthenticatedUser(request, primaryAuthResult.getResponse(), primaryAuthUser, true); - - if (session.isNew() && !primaryAuthUser.isGuest()) + if (setSession) { - // notify the websocket clients a new http session for the user has been started - NotificationService.get().sendServerEvent(primaryAuthUser.getUserId(), AuthNotify.LoggedIn); + // Prep the new session and set the user & authentication-related attributes + session = SecurityManager.setAuthenticatedUser(request, primaryAuthResult.getResponse(), primaryAuthUser, true); + + if (session.isNew() && !primaryAuthUser.isGuest()) + { + // notify the websocket clients a new http session for the user has been started + NotificationService.get().sendServerEvent(primaryAuthUser.getUserId(), AuthNotify.LoggedIn); + } } // Set the authentication validators into the new session diff --git a/api/src/org/labkey/api/security/LoginUrls.java b/api/src/org/labkey/api/security/LoginUrls.java index aa604fae023..73c35409786 100644 --- a/api/src/org/labkey/api/security/LoginUrls.java +++ b/api/src/org/labkey/api/security/LoginUrls.java @@ -15,6 +15,7 @@ */ package org.labkey.api.security; +import jakarta.servlet.http.HttpServletResponse; import org.jetbrains.annotations.Nullable; import org.labkey.api.action.UrlProvider; import org.labkey.api.data.Container; @@ -35,10 +36,11 @@ public interface LoginUrls extends UrlProvider ActionURL getLoginURL(URLHelper returnUrl); ActionURL getRegisterURL(Container c, @Nullable URLHelper returnUrl); ActionURL getLoginURL(Container c, @Nullable URLHelper returnUrl); - ActionURL getForceReauthURL(Container c, @Nullable URLHelper returnUrl); ActionURL getLogoutURL(Container c); ActionURL getLogoutURL(Container c, URLHelper returnUrl); ActionURL getStopImpersonatingURL(Container c, @Nullable URLHelper returnUrl); ActionURL getAgreeToTermsURL(Container c, URLHelper returnUrl); ActionURL getSSORedirectURL(SSOAuthenticationConfiguration configuration, URLHelper returnUrl, boolean skipProfile); + + void forceReauth(HttpServletResponse response, Container c, @Nullable URLHelper returnUrl); } diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index b56c2f77c50..0434f794d78 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -939,7 +939,7 @@ else if (ex instanceof UnauthorizedException uae) if (isGET) { // If user has not logged in or agreed to terms, not really unauthorized yet... - if (!isCSRFViolation && type.isRedirect(isGuest) && !overrideBasicAuth && HttpView.hasCurrentView()) + if (!isCSRFViolation && isGuest && type == UnauthorizedException.Type.redirectToLogin && !overrideBasicAuth && HttpView.hasCurrentView()) { // Issue 43307: If this is a locked project then just show the login page in the root. Register, // forgot my password, profile update, password reset, etc. aren't going to work right in a locked @@ -949,9 +949,7 @@ else if (ex instanceof UnauthorizedException uae) c = ContainerManager.getRoot(); // Issue 43387 - Retain original container info on login redirect URL, even if it resolved to something else - LoginUrls urls = PageFlowUtil.urlProvider(LoginUrls.class); - URLHelper returnURL = HttpView.getContextURLHelper(); - ActionURL loginURL = (type == UnauthorizedException.Type.redirectToLogin ? urls.getLoginURL(c, returnURL) : urls.getForceReauthURL(c, returnURL)); + ActionURL loginURL = PageFlowUtil.urlProvider(LoginUrls.class).getLoginURL(c, HttpView.getContextURLHelper()); Path originalContainerPath = (Path)request.getAttribute(ViewServlet.ORIGINAL_URL_CONTAINER_PATH); if (originalContainerPath != null) { diff --git a/api/src/org/labkey/api/view/ForceReauthException.java b/api/src/org/labkey/api/view/ForceReauthException.java deleted file mode 100644 index 8d4f20b2e01..00000000000 --- a/api/src/org/labkey/api/view/ForceReauthException.java +++ /dev/null @@ -1,13 +0,0 @@ -package org.labkey.api.view; - -// Use a subclass to prevent permission-checking code paths from setting the wrong type -public class ForceReauthException extends UnauthorizedException -{ - public static final String FORCE_REAUTH_NAME = "_forceReauth"; - - @Override - public Type getType() - { - return Type.forceReauth; - } -} diff --git a/api/src/org/labkey/api/view/UnauthorizedException.java b/api/src/org/labkey/api/view/UnauthorizedException.java index d44e15d89cc..d0ebd903d46 100644 --- a/api/src/org/labkey/api/view/UnauthorizedException.java +++ b/api/src/org/labkey/api/view/UnauthorizedException.java @@ -15,11 +15,10 @@ */ package org.labkey.api.view; +import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.Nullable; -import jakarta.servlet.http.HttpServletResponse; - /** * Signals to the HTTP client that the request is not authorized, via a 401 status code. */ @@ -30,33 +29,12 @@ public class UnauthorizedException extends HttpStatusException /** Options for how the client should be informed of not being allowed to see a resource */ public enum Type { - /** Always redirect the browser to the login page to force re-authentication */ - forceReauth() - { - @Override - public boolean isRedirect(boolean isGuest) - { - return true; - } - }, - /** If the user is guest, redirect the browser to render the login page */ - redirectToLogin() - { - @Override - public boolean isRedirect(boolean isGuest) - { - return isGuest; - } - }, + /** If user is guest, redirect the browser to the login page */ + redirectToLogin, /** Send a 401, but signal that the server would accept HTTP BasicAuth credentials */ - sendBasicAuth(), + sendBasicAuth, /** Send a 401 and don't solicit BasicAuth credentials */ - sendUnauthorized(); - - public boolean isRedirect(boolean isGuest) - { - return false; - } + sendUnauthorized } Type _type = Type.redirectToLogin; diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 960a41d7713..946855b5370 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -20,6 +20,7 @@ import jakarta.mail.MessagingException; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Logger; @@ -92,6 +93,7 @@ import org.labkey.api.settings.WriteableLookAndFeelProperties; import org.labkey.api.util.CSRFUtil; import org.labkey.api.util.ConfigurationException; +import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.HelpTopic; import org.labkey.api.util.HtmlString; @@ -105,7 +107,6 @@ import org.labkey.api.view.ActionURL; import org.labkey.api.view.BadRequestException; import org.labkey.api.view.ExternalRedirectException; -import org.labkey.api.view.ForceReauthException; import org.labkey.api.view.HtmlView; import org.labkey.api.view.HttpView; import org.labkey.api.view.JspView; @@ -248,13 +249,13 @@ public ActionURL getLoginURL(Container c, @Nullable URLHelper returnUrl) } @Override - public ActionURL getForceReauthURL(Container c, @Nullable URLHelper returnUrl) + public void forceReauth(HttpServletResponse response, Container c, @Nullable URLHelper returnUrl) { - if (returnUrl != null) - returnUrl.deleteParameter(ForceReauthException.FORCE_REAUTH_NAME); - - return getLoginURL(c, returnUrl) + ActionURL login = getLoginURL(c, returnUrl) .addParameter("forceReauth", true); + + // This method is called by CasServlet, so we don't have the luxury of throwing RedirectException, etc. + ExceptionUtil.unsafeRedirect(response, login.getLocalURIString()); } @Override @@ -689,7 +690,10 @@ public Object execute(LoginForm form, BindException errors) if (success) { - AuthenticationResult authResult = AuthenticationManager.handleAuthentication(request, getContainer()); + // We're never setting a session user in the force re-auth case (e.g., CAS renew=true), even if the + // there's no existing session. That seems in the spirit of re-auth, but we could go the other way + // and add "|| isGuest" to the last parameter. + AuthenticationResult authResult = AuthenticationManager.handleAuthentication(request, getContainer(), !form.isForceReauth()); // getUser will return null if authentication is incomplete as is the case when secondary authentication is required User user = authResult.getUser(); URLHelper redirectUrl = authResult.getRedirectURL(); From 02aaf24c91dd0541bc68add26aee6fbe1ae3a2f8 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 24 May 2026 13:57:34 -0700 Subject: [PATCH 08/13] Simplify CasServlet. Push handling from CAS servlet into CAS action. Set session to re-auth user when current session is guest. --- api/src/org/labkey/api/security/LoginUrls.java | 4 +--- .../org/labkey/core/login/LoginController.java | 18 +++++++----------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/api/src/org/labkey/api/security/LoginUrls.java b/api/src/org/labkey/api/security/LoginUrls.java index 73c35409786..aa604fae023 100644 --- a/api/src/org/labkey/api/security/LoginUrls.java +++ b/api/src/org/labkey/api/security/LoginUrls.java @@ -15,7 +15,6 @@ */ package org.labkey.api.security; -import jakarta.servlet.http.HttpServletResponse; import org.jetbrains.annotations.Nullable; import org.labkey.api.action.UrlProvider; import org.labkey.api.data.Container; @@ -36,11 +35,10 @@ public interface LoginUrls extends UrlProvider ActionURL getLoginURL(URLHelper returnUrl); ActionURL getRegisterURL(Container c, @Nullable URLHelper returnUrl); ActionURL getLoginURL(Container c, @Nullable URLHelper returnUrl); + ActionURL getForceReauthURL(Container c, @Nullable URLHelper returnUrl); ActionURL getLogoutURL(Container c); ActionURL getLogoutURL(Container c, URLHelper returnUrl); ActionURL getStopImpersonatingURL(Container c, @Nullable URLHelper returnUrl); ActionURL getAgreeToTermsURL(Container c, URLHelper returnUrl); ActionURL getSSORedirectURL(SSOAuthenticationConfiguration configuration, URLHelper returnUrl, boolean skipProfile); - - void forceReauth(HttpServletResponse response, Container c, @Nullable URLHelper returnUrl); } diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 946855b5370..37efb02fa30 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -20,7 +20,6 @@ import jakarta.mail.MessagingException; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Logger; @@ -93,7 +92,6 @@ import org.labkey.api.settings.WriteableLookAndFeelProperties; import org.labkey.api.util.CSRFUtil; import org.labkey.api.util.ConfigurationException; -import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.HelpTopic; import org.labkey.api.util.HtmlString; @@ -249,13 +247,10 @@ public ActionURL getLoginURL(Container c, @Nullable URLHelper returnUrl) } @Override - public void forceReauth(HttpServletResponse response, Container c, @Nullable URLHelper returnUrl) + public ActionURL getForceReauthURL(Container c, @Nullable URLHelper returnUrl) { - ActionURL login = getLoginURL(c, returnUrl) + return getLoginURL(c, returnUrl) .addParameter("forceReauth", true); - - // This method is called by CasServlet, so we don't have the luxury of throwing RedirectException, etc. - ExceptionUtil.unsafeRedirect(response, login.getLocalURIString()); } @Override @@ -690,10 +685,11 @@ public Object execute(LoginForm form, BindException errors) if (success) { - // We're never setting a session user in the force re-auth case (e.g., CAS renew=true), even if the - // there's no existing session. That seems in the spirit of re-auth, but we could go the other way - // and add "|| isGuest" to the last parameter. - AuthenticationResult authResult = AuthenticationManager.handleAuthentication(request, getContainer(), !form.isForceReauth()); + // Don't touch the session in the re-auth case (e.g., CAS renew=true) unless the user is guest. The + // CAS spec is silent on expected behavior when no "ticket-signing ticket" (session, in our case) + // exists and a "renew" is requested. Creating a new session seems valid and is an easy way to provide + // the authentication user on to validation actions (like cas-login). + AuthenticationResult authResult = AuthenticationManager.handleAuthentication(request, getContainer(), !form.isForceReauth() || isGuest); // getUser will return null if authentication is incomplete as is the case when secondary authentication is required User user = authResult.getUser(); URLHelper redirectUrl = authResult.getRedirectURL(); From ad196ff08d7442ea512c731663413ba79eb7f3fc Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 24 May 2026 14:05:48 -0700 Subject: [PATCH 09/13] Tweak comments --- core/src/org/labkey/core/login/LoginController.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 37efb02fa30..cd2d34a4429 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -685,10 +685,10 @@ public Object execute(LoginForm form, BindException errors) if (success) { - // Don't touch the session in the re-auth case (e.g., CAS renew=true) unless the user is guest. The - // CAS spec is silent on expected behavior when no "ticket-signing ticket" (session, in our case) - // exists and a "renew" is requested. Creating a new session seems valid and is an easy way to provide - // the authentication user on to validation actions (like cas-login). + // Don't touch the session in the re-auth case (e.g., CAS renew=true) except when the user is guest. + // The CAS spec is silent on expected behavior when no "ticket-signing ticket" (session, in our case) + // exists and a "renew" is requested. Creating a new session seems valid and provides a convenient way + // to convey the re-authed user to validation actions (like cas-login). AuthenticationResult authResult = AuthenticationManager.handleAuthentication(request, getContainer(), !form.isForceReauth() || isGuest); // getUser will return null if authentication is incomplete as is the case when secondary authentication is required User user = authResult.getUser(); From 33762732873f24c341e7f950790293d4883b59b2 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 24 May 2026 17:56:33 -0700 Subject: [PATCH 10/13] Stash re-auth user in session --- .../labkey/api/security/AuthenticationManager.java | 12 +++++++----- core/src/org/labkey/core/login/LoginController.java | 5 +++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index d8544297aab..3304cd6a9aa 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -1767,24 +1767,26 @@ public static URLHelper getWelcomeURL() return new URLHelper(true); } + public record Reauth(String token, User user){} public static final String REAUTH_TOKEN_NAME = "reauthToken"; - public static boolean reauthTokenMatches(HttpServletRequest request, String token) + public static @Nullable User getAndClearReauthUser(HttpServletRequest request, String token) { if (token == null) - return false; + return null; HttpSession session = request.getSession(false); if (session == null) - return false; + return null; - boolean matches = token.equals(session.getAttribute(REAUTH_TOKEN_NAME)); + Reauth reauth = (Reauth) session.getAttribute(REAUTH_TOKEN_NAME); + boolean matches = token.equals(reauth.token()); if (matches) session.removeAttribute(REAUTH_TOKEN_NAME); - return matches; + return reauth.user(); } // test() method should return true if the authentication is still valid diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index cd2d34a4429..b8a9fde78c3 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -60,6 +60,7 @@ import org.labkey.api.security.AuthenticationManager.AuthenticationStatus; import org.labkey.api.security.AuthenticationManager.LoginReturnProperties; import org.labkey.api.security.AuthenticationManager.PrimaryAuthenticationResult; +import org.labkey.api.security.AuthenticationManager.Reauth; import org.labkey.api.security.AuthenticationProvider; import org.labkey.api.security.AuthenticationProvider.SSOAuthenticationProvider; import org.labkey.api.security.CSRF; @@ -689,7 +690,7 @@ public Object execute(LoginForm form, BindException errors) // The CAS spec is silent on expected behavior when no "ticket-signing ticket" (session, in our case) // exists and a "renew" is requested. Creating a new session seems valid and provides a convenient way // to convey the re-authed user to validation actions (like cas-login). - AuthenticationResult authResult = AuthenticationManager.handleAuthentication(request, getContainer(), !form.isForceReauth() || isGuest); + AuthenticationResult authResult = AuthenticationManager.handleAuthentication(request, getContainer(), !form.isForceReauth()); // getUser will return null if authentication is incomplete as is the case when secondary authentication is required User user = authResult.getUser(); URLHelper redirectUrl = authResult.getRedirectURL(); @@ -709,7 +710,7 @@ else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE) { String reauthToken = GUID.makeHash(); redirectUrl.addParameter(REAUTH_TOKEN_NAME, reauthToken); - request.getSession().setAttribute(REAUTH_TOKEN_NAME, reauthToken); + request.getSession().setAttribute(REAUTH_TOKEN_NAME, new Reauth(reauthToken, user)); } // Use the full hostname in the URL if we have one, otherwise just go with a local URI From 4040e0524ae54e4ce92d7a42239a8a69483d28df Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 24 May 2026 19:24:06 -0700 Subject: [PATCH 11/13] Cleanup getAndClearReauthUser() --- .../api/security/AuthenticationManager.java | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 3304cd6a9aa..74e20dba6e5 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -1770,23 +1770,30 @@ public static URLHelper getWelcomeURL() public record Reauth(String token, User user){} public static final String REAUTH_TOKEN_NAME = "reauthToken"; - public static @Nullable User getAndClearReauthUser(HttpServletRequest request, String token) + public static @Nullable User getAndClearReauthUser(HttpServletRequest request, @Nullable String token) { - if (token == null) - return null; - - HttpSession session = request.getSession(false); + if (token != null) + { + HttpSession session = request.getSession(false); - if (session == null) - return null; + if (session != null) + { + Reauth reauth = (Reauth) session.getAttribute(REAUTH_TOKEN_NAME); - Reauth reauth = (Reauth) session.getAttribute(REAUTH_TOKEN_NAME); - boolean matches = token.equals(reauth.token()); + if (reauth != null) + { + boolean matches = token.equals(reauth.token()); - if (matches) - session.removeAttribute(REAUTH_TOKEN_NAME); + if (matches) + { + session.removeAttribute(REAUTH_TOKEN_NAME); + return reauth.user(); + } + } + } + } - return reauth.user(); + return null; } // test() method should return true if the authentication is still valid From fd99c2265799ac18c5c5dd8d7ced0b1ef99dc645 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 24 May 2026 19:28:29 -0700 Subject: [PATCH 12/13] Update comment --- core/src/org/labkey/core/login/LoginController.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index b8a9fde78c3..7185d60451d 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -686,10 +686,9 @@ public Object execute(LoginForm form, BindException errors) if (success) { - // Don't touch the session in the re-auth case (e.g., CAS renew=true) except when the user is guest. - // The CAS spec is silent on expected behavior when no "ticket-signing ticket" (session, in our case) - // exists and a "renew" is requested. Creating a new session seems valid and provides a convenient way - // to convey the re-authed user to validation actions (like cas-login). + // Don't touch the session in the re-auth case (e.g., CAS renew=true). The CAS spec is silent on + // expected behavior when no "ticket-signing ticket" (session, in our case) exists and a "renew" is + // requested, but this seems consistent with "ignore the current session" when renew is requested. AuthenticationResult authResult = AuthenticationManager.handleAuthentication(request, getContainer(), !form.isForceReauth()); // getUser will return null if authentication is incomplete as is the case when secondary authentication is required User user = authResult.getUser(); From d7e3564349609a351763c850d4bf26d6377b0c5b Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 25 May 2026 10:29:12 -0700 Subject: [PATCH 13/13] getRequestOrThrow(). Stash cas/login for login redirect. --- .../org/labkey/api/security/AuthenticationManager.java | 2 +- api/src/org/labkey/api/view/ViewContext.java | 7 +++++++ core/src/org/labkey/core/login/LoginController.java | 8 ++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 74e20dba6e5..cff56039bf1 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -558,7 +558,7 @@ public ModelAndView getView(FORM form, BindException errors) throws Exception } else { - HttpServletRequest request = getViewContext().getRequest(); + HttpServletRequest request = getViewContext().getRequestOrThrow(); final PrimaryAuthenticationResult primaryResult; diff --git a/api/src/org/labkey/api/view/ViewContext.java b/api/src/org/labkey/api/view/ViewContext.java index ce54015ca89..400a3a9bbed 100644 --- a/api/src/org/labkey/api/view/ViewContext.java +++ b/api/src/org/labkey/api/view/ViewContext.java @@ -224,6 +224,13 @@ public HttpServletRequest getRequest() return _request; } + public @NotNull HttpServletRequest getRequestOrThrow() + { + if (null == _request) + throw new IllegalStateException("Request is null"); + + return _request; + } public void setRequest(HttpServletRequest request) { diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 7185d60451d..97ff33e7647 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -744,7 +744,7 @@ else if (!errors.hasErrors()) response = new ApiSimpleResponse(); response.put("success", false); response.put(ActionURL.Param.returnUrl.name(), redirectString); - AuthenticationManager.setLoginReturnProperties(getViewContext().getRequest(), null); + AuthenticationManager.setLoginReturnProperties(getViewContext().getRequestOrThrow(), null); } } @@ -1582,7 +1582,7 @@ public ModelAndView getView(SsoRedirectForm form, BindException errors) if (null != returnUrl || form.getSkipProfile()) { LoginReturnProperties properties = new LoginReturnProperties(returnUrl, form.getUrlhash(), form.getSkipProfile()); - AuthenticationManager.setLoginReturnProperties(getViewContext().getRequest(), properties); + AuthenticationManager.setLoginReturnProperties(getViewContext().getRequestOrThrow(), properties); } final URLHelper url; @@ -1666,7 +1666,7 @@ buttonText, getTitle() if (null != returnUrl || form.getSkipProfile()) { LoginReturnProperties properties = new LoginReturnProperties(returnUrl, form.getUrlhash(), form.getSkipProfile()); - AuthenticationManager.setLoginReturnProperties(getViewContext().getRequest(), properties); + AuthenticationManager.setLoginReturnProperties(getViewContext().getRequestOrThrow(), properties); } // If we're going to display the form, then set focus on the first input. @@ -2210,7 +2210,7 @@ public ModelAndView getView(LoginForm form, boolean reshow, BindException errors if (null == form.getEmail()) { - form.setEmail(getEmailFromCookie(getViewContext().getRequest())); + form.setEmail(getEmailFromCookie(getViewContext().getRequestOrThrow())); } return view;