diff --git a/api/src/org/labkey/api/collections/ConcurrentSetValuedMap.java b/api/src/org/labkey/api/collections/ConcurrentSetValuedMap.java new file mode 100644 index 00000000000..c4c9838b2f7 --- /dev/null +++ b/api/src/org/labkey/api/collections/ConcurrentSetValuedMap.java @@ -0,0 +1,25 @@ +package org.labkey.api.collections; + +import org.apache.commons.collections4.multimap.AbstractSetValuedMap; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +// A concurrent multivalued map that's appropriate when each key is mapped to a small number of values +public class ConcurrentSetValuedMap extends AbstractSetValuedMap +{ + public ConcurrentSetValuedMap() + { + super(new ConcurrentHashMap<>()); + } + + @Override + protected Set createCollection() + { + // Be sure to review all usages of ConcurrentSetValuedMap before changing this implementation. Callers + // currently expect the behavior of synchronizedSet(). + return Collections.synchronizedSet(new HashSet<>(5)); + } +} diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 56a25bbc0f3..1537831b5bb 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -1265,22 +1265,25 @@ public static URLHelper logout(@NotNull User user, @NotNull HttpServletRequest r { addAuditEvent(user, request, user.getEmail() + " " + UserManager.UserAuditEvent.LOGGED_OUT + "."); - Integer configurationId = (Integer)session.getAttribute(SecurityManager.PRIMARY_AUTHENTICATION_CONFIGURATION); + PrimaryAuthenticationConfiguration configuration = getConfiguration(session); - if (null != configurationId) + if (null != configuration) { - PrimaryAuthenticationConfiguration configuration = AuthenticationConfigurationCache.getConfiguration(PrimaryAuthenticationConfiguration.class, configurationId); - - if (null != configuration) - { - ret = configuration.logout(request, returnURL); - } + ret = configuration.logout(request, returnURL); } } return ret; } + // Returns null if configuration ID is not present (user not logged in) or configuration no longer exists + public static @Nullable PrimaryAuthenticationConfiguration getConfiguration(@NotNull HttpSession session) + { + Integer configurationId = (Integer) session.getAttribute(SecurityManager.PRIMARY_AUTHENTICATION_CONFIGURATION); + + return null != configurationId ? AuthenticationConfigurationCache.getConfiguration(PrimaryAuthenticationConfiguration.class, configurationId) : null; + } + /** * @return A case-insensitive map of user attribute names and values that was stashed in the associated session at * authentication time. This map will often be empty but will never be null. diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index d32c13bc945..2e379d92dbb 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -533,7 +533,7 @@ public static User getSessionUser(HandshakeRequest request) return sessionUser; } - public static User getSessionUser(HttpSession session) + public static @Nullable User getSessionUser(@Nullable HttpSession session) { User sessionUser = null; diff --git a/api/src/org/labkey/api/security/UserManager.java b/api/src/org/labkey/api/security/UserManager.java index 131c3129d2a..ec8cb16155e 100644 --- a/api/src/org/labkey/api/security/UserManager.java +++ b/api/src/org/labkey/api/security/UserManager.java @@ -19,7 +19,9 @@ import jakarta.servlet.http.HttpSession; import jakarta.servlet.http.HttpSessionEvent; import jakarta.servlet.http.HttpSessionListener; +import org.apache.commons.collections4.MultiValuedMap; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.mutable.MutableInt; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -29,6 +31,7 @@ import org.junit.Test; import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.AuditTypeEvent; +import org.labkey.api.collections.ConcurrentSetValuedMap; import org.labkey.api.data.Aggregate; import org.labkey.api.data.ButtonBar; import org.labkey.api.data.ColumnInfo; @@ -66,6 +69,7 @@ import org.labkey.api.util.Link; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; +import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.TestContext; import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; @@ -488,14 +492,14 @@ public static int getAuthCount(@Nullable Date since, boolean excludeSystemUsers, { sql.append(" INNER JOIN "); sql.append(CoreSchema.getInstance().getTableInfoUsersData(), "ud"); - sql.append(" ON uat.CreatedBy = ud.UserId "); - sql.append(" WHERE ud.System = ? "); + sql.append(" ON uat.CreatedBy = ud.UserId"); + sql.append(" WHERE ud.System = ?"); sql.add(false); } else { // Make string concat easy - sql.append(" WHERE 1=1 "); + sql.append(" WHERE 1=1"); } sql.append(" AND uat.Comment LIKE "); @@ -522,7 +526,7 @@ public static int getAuthCount(@Nullable Date since, boolean excludeSystemUsers, /** In minutes */ private static final AtomicLong _totalSessionDuration = new AtomicLong(); - private static final Set _activeSessions = Collections.synchronizedSet(new HashSet<>()); + private static final MultiValuedMap _activeSessions = new ConcurrentSetValuedMap<>(); public static void ensureSessionTracked(HttpSession s) { @@ -530,7 +534,10 @@ public static void ensureSessionTracked(HttpSession s) { Integer userId = (Integer)s.getAttribute(USER_ID_KEY); if (null != userId && getGuestUser().getUserId() != userId) - _activeSessions.add(s.getId()); + { + if (_activeSessions.put(userId, s)) + LOG.debug("Tracking a new session. {} active.", StringUtilsLabKey.pluralize(getActiveUserSessionCount(), "session")); + } } } @@ -545,25 +552,73 @@ public void sessionCreated(HttpSessionEvent event) @Override public void sessionDestroyed(HttpSessionEvent event) { - _activeSessions.remove(event.getSession().getId()); - // Issue 44761 - track session duration for authenticated users User user = SecurityManager.getSessionUser(event.getSession()); if (user != null) { + _activeSessions.removeMapping(user.getUserId(), event.getSession()); + long duration = TimeUnit.MILLISECONDS.toMinutes(event.getSession().getLastAccessedTime() - event.getSession().getCreationTime()); - LOG.debug("Adding session duration to tally for " + user.getEmail() + ", " + duration + " minutes"); + LOG.debug("Destroyed session for {}. Adding session duration of {} minutes to tally. {} active.", user.getEmail(), duration, StringUtilsLabKey.pluralize(getActiveUserSessionCount(), "session")); _sessionCount.incrementAndGet(); _totalSessionDuration.addAndGet(duration); } } } + public static void terminateAllSessionsForUser(@Nullable User user) + { + handleSessionsForUser(user, new SessionHandler() + { + @Override + public boolean handleSession(HttpSession session) + { + session.invalidate(); + return true; + } + + @Override + public void complete(int count) + { + //noinspection DataFlowIssue + LOG.debug("Invalidated {} for {}.", StringUtilsLabKey.pluralize(count, "session"), user.getEmail()); + } + }); + } + + public interface SessionHandler + { + // Return true to increment the count passed to complete() after handleSession() has been invoked for each active session + boolean handleSession(HttpSession session); + // Called after all sessions have been passed to handleSession(), only if user is non-null and was logged in. + // Useful for coalescing logging, for example. + void complete(int count); + } + + public static void handleSessionsForUser(@Nullable User user, SessionHandler handler) + { + if (user != null && !user.isGuest()) + { + MutableInt count = new MutableInt(); + + // The SessionHandler may directly or indirectly mutate this user's session set, so enumerate a copy to avoid + // concurrent modification exceptions. + Set sessions = new HashSet<>(_activeSessions.get(user.getUserId())); + sessions.forEach(session -> { + if (handler.handleSession(session)) + count.increment(); + }); + + handler.complete(count.intValue()); + } + } + public static int getActiveUserSessionCount() { return _activeSessions.size(); } + // Average duration of all sessions that have ended public static Integer getAverageSessionDuration() { return _sessionCount.get() == 0 ? null : (int)(_totalSessionDuration.get() / _sessionCount.get()); diff --git a/core/src/org/labkey/core/login/DbLoginManager.java b/core/src/org/labkey/core/login/DbLoginManager.java index f2bc3ad22d8..f18abdd8eca 100644 --- a/core/src/org/labkey/core/login/DbLoginManager.java +++ b/core/src/org/labkey/core/login/DbLoginManager.java @@ -16,13 +16,16 @@ package org.labkey.core.login; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpSession; import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.labkey.api.audit.AuditLogService; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.PropertyManager; import org.labkey.api.data.PropertyManager.WritablePropertyMap; +import org.labkey.api.security.AuthenticationConfiguration.PrimaryAuthenticationConfiguration; import org.labkey.api.security.AuthenticationConfigurationCache; import org.labkey.api.security.AuthenticationManager; import org.labkey.api.security.AuthenticationManager.AuthenticationResult; @@ -33,6 +36,7 @@ import org.labkey.api.security.SecurityManager; import org.labkey.api.security.User; import org.labkey.api.security.UserManager; +import org.labkey.api.security.UserManager.SessionHandler; import org.labkey.api.security.ValidEmail; import org.labkey.api.security.ValidEmail.InvalidEmailException; import org.labkey.api.settings.LookAndFeelProperties; @@ -40,6 +44,7 @@ import org.labkey.api.usageMetrics.UsageMetricsProvider; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.URLHelper; +import org.labkey.api.util.logging.LogHelper; import org.labkey.core.login.LoginController.SaveDbLoginPropertiesForm; import org.springframework.validation.BindException; @@ -54,6 +59,8 @@ public class DbLoginManager implements DbLoginService { // TODO: Move Logins table operations here + private static final Logger LOG = LogHelper.getLogger(DbLoginManager.class, "Database login information"); + public static DbLoginConfiguration getConfiguration() { Collection configurations = AuthenticationManager.getActiveConfigurations(DbLoginConfiguration.class); @@ -95,6 +102,30 @@ public AuthenticationResult attemptSetPassword(Container c, User currentUser, St try { SecurityManager.setPassword(email, password); + // Invalidate all sessions belonging to this user that were authenticated via database authentication + UserManager.handleSessionsForUser(user, new SessionHandler() + { + @Override + public boolean handleSession(HttpSession session) + { + PrimaryAuthenticationConfiguration configuration = AuthenticationManager.getConfiguration(session); + + if (configuration instanceof DbLoginConfiguration) + { + session.invalidate(); + return true; + } + + return false; + } + + @Override + public void complete(int count) + { + //noinspection DataFlowIssue + LOG.debug("Invalidated {} for {}.", StringUtilsLabKey.pluralize(count, "session"), user.getEmail()); + } + }); } catch (SecurityManager.UserManagementException e) { @@ -114,9 +145,10 @@ public AuthenticationResult attemptSetPassword(Container c, User currentUser, St return null; } - // Should log user in only for initial user, choose password, and forced change password scenarios, but not for scenarios - // where a user is already logged in (normal change password, admins initializing another user's password, etc.) - if (currentUser.isGuest()) + // The affected user's sessions were all terminated above. If the request's session doesn't exist or is a guest + // session then log the user in using the just-entered credentials. An admin resetting a password won't be + // affected. This check should be equivalent to currentUser.equals(user). + if (SecurityManager.getSessionUser(request.getSession()) == null) { AuthenticationManager.PrimaryAuthenticationResult result = AuthenticationManager.authenticate(request, email.getEmailAddress(), password, returnUrlHelper, true); diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 14d1ec75fa2..3e436a70478 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -1696,6 +1696,9 @@ protected boolean clearVerification() private AuthenticationResult attemptSetPassword(ValidEmail email, URLHelper returnUrlHelper, String auditMessage, boolean clearVerification, BindException errors) throws InvalidEmailException { HttpServletRequest request = getViewContext().getRequest(); + // TODO: This is unreliable... e.g., the "change password via email validation" scenario isn't considered a + // change, so messaging and audit text is off. Actions should explicitly determine whether this is a change + // operation vs. an initial password. Need a flag or perhaps separate actions. boolean changeOperation = StringUtils.startsWithIgnoreCase(auditMessage, "change"); return DbLoginService.get().attemptSetPassword(getContainer(), getUser(), request.getParameter("password"), request.getParameter("password2"), request, email, returnUrlHelper, auditMessage, clearVerification, changeOperation, errors);