Invalidate sessions on password change (#5994)

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<K, V> extends AbstractSetValuedMap<K, V>
+{
+    public ConcurrentSetValuedMap()
+    {
+        super(new ConcurrentHashMap<>());
+    }
+
+    @Override
+    protected Set<V> 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));
+    }
+}

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.

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;
 

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,15 +526,18 @@ public static int getAuthCount(@Nullable Date since, boolean excludeSystemUsers,
     /** In minutes */
     private static final AtomicLong _totalSessionDuration = new AtomicLong();
 
-    private static final Set<String> _activeSessions = Collections.synchronizedSet(new HashSet<>());
+    private static final MultiValuedMap<Integer, HttpSession> _activeSessions = new ConcurrentSetValuedMap<>();
 
     public static void ensureSessionTracked(HttpSession s)
     {
         if (s != null)
         {
             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<HttpSession> 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());

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,13 +36,15 @@
 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;
 import org.labkey.api.settings.StartupProperty;
 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<DbLoginConfiguration> 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);
 

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);