Skip to content

Commit 3707b50

Browse files
committed
Revert the fix that preserved reauth through Duo/TOTP callbacks, and make the reauth complete after primary credential verification only as per 21 CFR Part 11.
1 parent 74d990d commit 3707b50

2 files changed

Lines changed: 40 additions & 26 deletions

File tree

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

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,9 +1605,8 @@ private static String getAuthenticationProcessSessionKey(int configurationId)
16051605
}
16061606

16071607
// Session-scoped marker that a forceReauth flow is in progress. Set by LoginApiAction before primary auth and
1608-
// consumed by handleAuthentication after secondary auth completes, so the reauth token can be issued without
1609-
// replacing the existing authenticated session. Survives the round-trip to external 2FA providers (Duo, TOTP),
1610-
// which is essential because their validate actions call handleAuthentication without form context.
1608+
// consumed by handleAuthentication after primary auth succeeds, so the reauth token can be issued without
1609+
// replacing the existing authenticated session or running the full login completion flow.
16111610
private record ReauthFlow(boolean local) {}
16121611

16131612
private static String getReauthFlowSessionKey()
@@ -1620,14 +1619,6 @@ public static void setReauthFlow(HttpServletRequest request, boolean local)
16201619
request.getSession(true).setAttribute(getReauthFlowSessionKey(), new ReauthFlow(local));
16211620
}
16221621

1623-
// Used by 2FA validate actions to tell whether the already-logged-in user is mid-reauth, so they don't
1624-
// short-circuit to the home page and skip the reauth-token issuance in handleAuthentication.
1625-
public static boolean isReauthInProgress(HttpServletRequest request)
1626-
{
1627-
HttpSession session = request.getSession(false);
1628-
return session != null && session.getAttribute(getReauthFlowSessionKey()) != null;
1629-
}
1630-
16311622
// Clear all primary and secondary authentication results
16321623
public static void clearAuthenticationProcessAttributes(HttpServletRequest request)
16331624
{
@@ -1705,6 +1696,17 @@ public URLHelper getRedirectURL()
17051696
}
17061697
}
17071698

1699+
// As per 21 CFR Part 11, reauth only requires primary credentials (email + password), so issue the token
1700+
// before normal login completion (secondary auth, profile-update redirects, validators, or session replacement).
1701+
ReauthFlow reauthFlow = (ReauthFlow)session.getAttribute(getReauthFlowSessionKey());
1702+
if (reauthFlow != null)
1703+
{
1704+
session.removeAttribute(getReauthFlowSessionKey());
1705+
URLHelper url = getAfterReauthURL(c, getLoginReturnProperties(request), primaryAuthUser);
1706+
setReauthUser(primaryAuthUser, reauthFlow.local() ? SecurityManager.getSessionUser(request) : null, request, null, url);
1707+
return new AuthenticationResult(primaryAuthUser, url);
1708+
}
1709+
17081710
List<AuthenticationValidator> validators = new LinkedList<>();
17091711

17101712
if (primaryAuthResult.getResponse().requireSecondary())
@@ -1753,17 +1755,6 @@ public URLHelper getRedirectURL()
17531755
LoginReturnProperties properties = getLoginReturnProperties(request);
17541756
URLHelper url = getAfterLoginURL(c, properties, primaryAuthUser);
17551757

1756-
// Reauth path: primary (and any secondary) auth has completed. Issue the one-time reauth token on the return
1757-
// URL and bail out *before* setAuthenticatedUser runs, so the user's existing session is preserved. The local
1758-
// flag distinguishes local login-page reauth (must match current session user) from CAS IdP reauth (any user).
1759-
ReauthFlow reauthFlow = (ReauthFlow)session.getAttribute(getReauthFlowSessionKey());
1760-
if (reauthFlow != null)
1761-
{
1762-
session.removeAttribute(getReauthFlowSessionKey());
1763-
setReauthUser(primaryAuthUser, reauthFlow.local() ? SecurityManager.getSessionUser(request) : null, request, null, url);
1764-
return new AuthenticationResult(primaryAuthUser, url);
1765-
}
1766-
17671758
if (setSession)
17681759
{
17691760
// Prep the new session and set the user & authentication-related attributes
@@ -1785,6 +1776,30 @@ public URLHelper getRedirectURL()
17851776
return new AuthenticationResult(primaryAuthUser, url);
17861777
}
17871778

1779+
// Builds the URL that receives the reauth token from setReauthUser().
1780+
// Unlike getAfterLoginURL(), this keeps the caller's return URL even if the user needs a profile update.
1781+
private static URLHelper getAfterReauthURL(Container current, @Nullable LoginReturnProperties properties, @NotNull User user)
1782+
{
1783+
URLHelper returnUrl;
1784+
1785+
if (null != properties && null != properties.getReturnUrl())
1786+
{
1787+
returnUrl = properties.getReturnUrl();
1788+
}
1789+
else
1790+
{
1791+
Container c = (null == current || current.isRoot() ? ContainerManager.getHomeContainer() : current);
1792+
returnUrl = !c.hasPermission(user, ReadPermission.class) ? getWelcomeURL() : c.getStartURL(user);
1793+
}
1794+
1795+
if (null != properties && null != properties.getUrlhash())
1796+
{
1797+
returnUrl.setFragment(properties.getUrlhash().replace("#", ""));
1798+
}
1799+
1800+
return returnUrl;
1801+
}
1802+
17881803

17891804
public static URLHelper getAfterLoginURL(Container current, @Nullable LoginReturnProperties properties, @NotNull User user)
17901805
{

core/src/org/labkey/core/login/LoginController.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ public Object execute(LoginForm form, BindException errors)
684684
Project termsProject = getTermsOfUseProject(form);
685685
boolean isGuest = getUser().isGuest();
686686

687-
if (!isTermsOfUseApproved(form) && !form.isApprovedTermsOfUse())
687+
if (!form.isForceReauth() && !isTermsOfUseApproved(form) && !form.isApprovedTermsOfUse())
688688
{
689689
if (null != termsProject)
690690
{
@@ -709,8 +709,7 @@ public Object execute(LoginForm form, BindException errors)
709709
// expected behavior when no "ticket-signing ticket" (session, in our case) exists and a "renew" is
710710
// requested, but this seems consistent with "ignore the current session" when renew is requested.
711711
// Stash the reauth context in session so handleAuthentication can issue the reauth token after
712-
// secondary auth completes. Session-scoped because secondary auth (Duo/TOTP) callbacks re-enter
713-
// handleAuthentication without form context.
712+
// primary authentication succeeds without running the full login completion flow.
714713
if (form.isForceReauth())
715714
AuthenticationManager.setReauthFlow(request, form.isLocal());
716715

@@ -721,7 +720,7 @@ public Object execute(LoginForm form, BindException errors)
721720
response = new ApiSimpleResponse();
722721
response.put("success", true);
723722

724-
if (form.isApprovedTermsOfUse())
723+
if (!form.isForceReauth() && form.isApprovedTermsOfUse())
725724
{
726725
if (form.getTermsOfUseType() == TermsOfUseType.PROJECT_LEVEL)
727726
WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), termsProject, true);

0 commit comments

Comments
 (0)