Merge remote-tracking branch 'origin/develop' into fb_entityCreateTyp…

…eChoice

api/src/org/labkey/api/assay/PreviouslyUploadedDataCollector.java

@@ -16,6 +16,7 @@
 
 package org.labkey.api.assay;
 
+import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.NotNull;
 import org.labkey.api.collections.CollectionUtils;
 import org.labkey.api.data.Container;
@@ -173,7 +174,7 @@ private Map<String, FileLike> getFilesFromRequest(ContextType context, Type type
         Map<String, FileLike> result = CollectionUtils.enforceValueClass(new LinkedHashMap<>(), FileLike.class);
         for (int i = 0; i < paths.length; i++)
         {
-            result.put(names[i], pipelineRoot.resolvePathToFileLike(paths[i]));
+            result.put(names[i], pipelineRoot.resolvePathToFileLike(StringUtils.replace(paths[i],"\\","/")));
         }
         return result;
     }

api/src/org/labkey/api/data/DbScope.java

@@ -1157,6 +1157,9 @@ public synchronized boolean release(Connection conn)
         {
             log(() -> 1 == _refCount ? "Releasing connection [1]: " + conn.toString() : "Attempting to decrease count of connection [" + _refCount + "]: " + conn.toString());
 
+            if (_conn == null)
+                throw new ConnectionAlreadyReleaseException("Connection has already been nulled out, but was passed: " + conn);
+
             if (_conn != conn)
                 throw new IllegalStateException("Incorrect Connection: " + conn + " vs. " + _conn);
 
@@ -1174,6 +1177,19 @@ public synchronized boolean release(Connection conn)
         }
     }
 
+    /**
+     * Special case for when the connection being closed doesn't match the expected
+     * connection for this thread, to help scenarios that are prone to race conditions
+     * (like killing pipeline jobs) ignore it.
+     */
+    public static class ConnectionAlreadyReleaseException extends IllegalStateException
+    {
+        public ConnectionAlreadyReleaseException(String s)
+        {
+            super(s);
+        }
+    }
+
     private void log(Supplier<String> supplier)
     {
         if (LOG.isDebugEnabled())
@@ -2080,7 +2096,6 @@ public static void clearFailedDbScopes()
      */
     public static void closeAllConnectionsForCurrentThread()
     {
-        Thread thread = getEffectiveThread();
         for (DbScope scope : getInitializedDbScopes())
         {
             TransactionImpl t = scope.getCurrentTransactionImpl();
@@ -2097,7 +2112,11 @@ public static void closeAllConnectionsForCurrentThread()
                 {
                     LOG.warn("Forcing close of still-pending transaction object. Current stack is ", new Throwable());
                     LOG.warn("Forcing close of still-pending transaction object started at ", t._creation);
-                    t.close(thread);
+                    t.close();
+                }
+                catch (ConnectionAlreadyReleaseException ignored)
+                {
+                    // The code in another thread has already released the connection
                 }
                 catch (Exception x)
                 {
@@ -2566,11 +2585,6 @@ public String getId()
 
         @Override
         public void close()
-        {
-            close(getEffectiveThread());
-        }
-
-        public void close(Thread thread)
         {
             if (_closesToIgnore == 0)
             {

api/src/org/labkey/api/data/TempTableTracker.java

@@ -163,11 +163,13 @@ private void untrack()
 
         synchronized(createdTableNames)
         {
-            createdTableNames.remove(qualifiedName);
+            var ttt = createdTableNames.remove(qualifiedName);
             appendToLog("-" + schemaName + "\t" + tableName + "\n");
 
             if (createdTableNames.isEmpty() || System.currentTimeMillis() > lastSync + CacheManager.DAY)
                 synchronizeLog(false);
+
+            ttt.clear();
         }
     }
 

api/src/org/labkey/api/jsp/LabKeyJspWriter.java

@@ -32,15 +32,25 @@ public class LabKeyJspWriter extends JspWriterWrapper
     }
 
     @Override
-    public void print(char[] s)
+    public void print(char[] c)
     {
-        throwException("A JSP is attempting to render a character array!");
+        String s = new String(c);
+        throwException("A JSP is attempting to render a character array! " + truncateAndQuote(s));
+    }
+
+    private String truncateAndQuote(String s)
+    {
+        if (s == null)
+        {
+            return null;
+        }
+        return "'" + (s.length() < 50 ? s : (s.substring(0, 50) + "...")) + "'";
     }
 
     @Override
     public void print(String s) throws IOException
     {
-        throwException("A JSP is attempting to render a string!");
+        throwException("A JSP is attempting to render a string! " + truncateAndQuote(s));
     }
 
     @Override

api/src/org/labkey/api/qc/TsvDataSerializer.java

@@ -122,7 +122,14 @@ else if (Collection.class.isAssignableFrom(o.getClass()))
                 else if (Object[].class.isAssignableFrom(o.getClass()))
                     pw.append(StringUtils.join((Object[]) o, ","));
                 else
-                    pw.append(String.valueOf(o));
+                {
+                    String val = String.valueOf(o);
+                    // Issue 51629 double quote the value if it contains backslashes to avoid tab loader mangling
+                    // on import
+                    if (!val.startsWith("\"") && !val.endsWith("\"") && val.contains("\\"))
+                        val = "\"" + StringUtils.replace(val,"\"", "\"\"") + "\"";
+                    pw.append(val);
+                }
             }
             sep = "\t";
         }

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

@@ -15,6 +15,7 @@
  */
 package org.labkey.api.security;
 
+import jakarta.servlet.http.HttpSession;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Logger;
 import org.jetbrains.annotations.NotNull;
@@ -25,6 +26,7 @@
 import org.labkey.api.data.DbScope;
 import org.labkey.api.data.DbScope.Transaction;
 import org.labkey.api.data.DbScope.TransactionKind;
+import org.labkey.api.data.Filter;
 import org.labkey.api.data.SQLFragment;
 import org.labkey.api.data.SimpleFilter;
 import org.labkey.api.data.SimpleFilter.FilterClause;
@@ -35,30 +37,37 @@
 import org.labkey.api.data.TableSelector;
 import org.labkey.api.module.ModuleLoader;
 import org.labkey.api.query.FieldKey;
+import org.labkey.api.security.UserManager.SessionHandler;
 import org.labkey.api.security.ValidEmail.InvalidEmailException;
 import org.labkey.api.settings.AppProps;
 import org.labkey.api.settings.LenientStartupPropertyHandler;
 import org.labkey.api.settings.StartupProperty;
 import org.labkey.api.settings.StartupPropertyEntry;
 import org.labkey.api.util.ConfigurationException;
 import org.labkey.api.util.GUID;
+import org.labkey.api.util.StringUtilsLabKey;
 import org.labkey.api.util.SystemMaintenance.MaintenanceTask;
 import org.labkey.api.util.TestContext;
+import org.labkey.api.util.logging.LogHelper;
 import org.labkey.api.view.UnauthorizedException;
 
 import java.time.Instant;
 import java.time.LocalDateTime;
 import java.time.ZoneId;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 
 public class ApiKeyManager
 {
-    private final static ApiKeyManager INSTANCE = new ApiKeyManager();
-    private final static TransactionKind TRANSACTION_KIND = () -> "APIKEY";
+    private static final Logger LOG = LogHelper.getLogger(ApiKeyManager.class, "API key bookkeeping");
+    private static final ApiKeyManager INSTANCE = new ApiKeyManager();
+    private static final TransactionKind TRANSACTION_KIND = () -> "APIKEY";
+
+    public static final String API_KEY_ROW_ID = "apiKeyRowID";
 
     public static ApiKeyManager get()
     {
@@ -118,17 +127,54 @@ private ApiKeyManager()
         return apiKey;
     }
 
-    public void deleteKey(String apikey)
+    // Delete a single API key and clear any active sessions that were established using it
+    public void deleteKey(String apiKey)
     {
-        SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Crypt"), crypt(apikey));
+        deleteKeys(new SimpleFilter(FieldKey.fromParts("Crypt"), crypt(apiKey)));
+    }
 
+    // Delete all API keys that match the filter and clear all active sessions that were established using them
+    public void deleteKeys(Filter filter)
+    {
         try (Transaction t = CoreSchema.getInstance().getScope().beginTransaction(TRANSACTION_KIND))
         {
-            Table.delete(CoreSchema.getInstance().getTableAPIKeys(), filter);
+            new TableSelector(CoreSchema.getInstance().getTableAPIKeys(), filter, null)
+                .forEach(ApiKeyAuthentication.class, auth -> deleteKey(auth.getUser(), auth.rowId()));
             t.commit();
         }
     }
 
+    // Delete a single API key and clear all active sessions that were established using it
+    private void deleteKey(User user, int rowId)
+    {
+        Table.delete(CoreSchema.getInstance().getTableAPIKeys(), rowId);
+        UserManager.handleSessionsForUser(user, new SessionHandler()
+        {
+            @Override
+            public boolean handleSession(HttpSession session)
+            {
+                LOG.debug("Checking if session {} used API key {}", session.getId(), rowId);
+                Map<String, Object> map = AuthenticationManager.getAuthenticationProperties(session);
+                Integer apiKeyRowId = (Integer)map.get(API_KEY_ROW_ID);
+
+                if (Objects.equals(rowId, apiKeyRowId))
+                {
+                    LOG.debug("Attempting to invalidate session {} which used API key {}", session.getId(), rowId);
+                    session.invalidate();
+                    return true;
+                }
+
+                return false;
+            }
+
+            @Override
+            public void complete(int count)
+            {
+                LOG.debug("Invalidated {} for {}.", StringUtilsLabKey.pluralize(count, "API key session"), user.getEmail());
+            }
+        });
+    }
+
     public void updateLastUsed(String apikey)
     {
         DbScope scope = CoreSchema.getInstance().getScope();
@@ -141,25 +187,34 @@ public void updateLastUsed(String apikey)
         }
     }
 
+    public record ApiKeyAuthentication(int createdBy, int rowId)
+    {
+        public User getUser()
+        {
+            return UserManager.getUser(createdBy());
+        }
+    }
+
     /**
-     * Returns the User associated with the supplied API key, if API key is valid. User could be inactive.
+     * Returns a record containing the User associated with the supplied API key and the API key's rowID, if API key is
+     * valid. User could be inactive.
      * @param apiKey The API key to validate
-     * @return The User associated with the API key or null if API key is invalid
+     * @return An ApiKeyAuthentication associated with the API key or null if API key is invalid
      */
-    public @Nullable User authenticateFromApiKey(@NotNull String apiKey)
+    public @Nullable ApiKeyAuthentication authenticateFromApiKey(@NotNull String apiKey)
     {
         SimpleFilter filter = new SimpleFilter(getStillValidClause());
         filter.addCondition(FieldKey.fromParts("Crypt"), crypt(apiKey));
 
-        Integer userId;
+        ApiKeyAuthentication ret;
 
         try (Transaction t = CoreSchema.getInstance().getScope().beginTransaction(TRANSACTION_KIND))
         {
-            userId = new TableSelector(CoreSchema.getInstance().getTableAPIKeys(), Collections.singleton("CreatedBy"), filter, null).getObject(Integer.class);
+            ret = new TableSelector(CoreSchema.getInstance().getTableAPIKeys(), Set.of("CreatedBy", "RowId"), filter, null).getObject(ApiKeyAuthentication.class);
             t.commit();
         }
 
-        return null != userId ? UserManager.getUser(userId) : null;
+        return ret;
     }
 
     private static final String API_KEY_SCOPE = "ApiKey";
@@ -217,7 +272,6 @@ private static FilterClause getStillValidClause()
         return new SQLClause(new SQLFragment("Expiration IS NULL OR Expiration > ?", new Date()), FieldKey.fromParts("Expiration"));
     }
 
-
     public static class TestCase extends Assert
     {
         @Test
@@ -227,11 +281,15 @@ public void testCreateAndExpire() throws InterruptedException
             String oneSecondKey = ApiKeyManager.get().createKey(user, 1, "Created by ApiKeyManager.TestCase");
             String noExpireKey = ApiKeyManager.get().createKey(user, -1, "Created by ApiKeyManager.TestCase");
 
-            assertEquals(user, ApiKeyManager.get().authenticateFromApiKey(oneSecondKey));
+            ApiKeyAuthentication auth = ApiKeyManager.get().authenticateFromApiKey(oneSecondKey);
+            assertNotNull(auth);
+            assertEquals(user, auth.getUser());
             Thread.sleep(1100);
             assertNull("API key should have expired after one second", ApiKeyManager.get().authenticateFromApiKey(oneSecondKey));
 
-            assertEquals(user, ApiKeyManager.get().authenticateFromApiKey(noExpireKey));
+            auth = ApiKeyManager.get().authenticateFromApiKey(noExpireKey);
+            assertNotNull(auth);
+            assertEquals(user, auth.getUser());
 
             ApiKeyManager.get().deleteKey(oneSecondKey);
             ApiKeyManager.get().deleteKey(noExpireKey);
@@ -255,10 +313,14 @@ public void testTransaction()
             try (Transaction ignored = DbScope.getLabKeyScope().beginTransaction())
             {
                 apikey = ApiKeyManager.get().createKey(user, 10, "Created by ApiKeyManager.TestCase");
-                assertEquals(user, ApiKeyManager.get().authenticateFromApiKey(apikey));
+                ApiKeyAuthentication auth = ApiKeyManager.get().authenticateFromApiKey(apikey);
+                assertNotNull(auth);
+                assertEquals(user, auth.getUser());
             }
 
-            assertEquals("API key should be valid even after rollback", user, ApiKeyManager.get().authenticateFromApiKey(apikey));
+            ApiKeyAuthentication auth = ApiKeyManager.get().authenticateFromApiKey(apikey);
+            assertNotNull(auth);
+            assertEquals("API key should be valid even after rollback", user, auth.getUser());
 
             ApiKeyManager.get().deleteKey(apikey);
             assertNull(ApiKeyManager.get().authenticateFromApiKey(apikey));
@@ -282,12 +344,8 @@ public String getName()
         @Override
         public void run(Logger log)
         {
-            try (Transaction t = CoreSchema.getInstance().getScope().beginTransaction(TRANSACTION_KIND))
-            {
-                // Delete all rows that are no longer valid (expired)
-                Table.delete(CoreSchema.getInstance().getTableAPIKeys(), new SimpleFilter(new NotClause(getStillValidClause())));
-                t.commit();
-            }
+            // Delete all rows that are no longer valid (expired)
+            ApiKeyManager.get().deleteKeys(new SimpleFilter(new NotClause(getStillValidClause())));
         }
 
         @Override

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

@@ -263,10 +263,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
             // across Tomcat restarts. Ensure that all authenticated users have their sessions tracked, so we can
             // accurately assess if anyone is logged in
             HttpSession s = req.getSession(false);
-            if (s != null && !user.isGuest())
-            {
-                UserManager.ensureSessionTracked(s);
-            }
+            UserManager.ensureSessionTracked(user, s);
 
             SecurityLogger.popSecurityContext();
             QueryService.get().clearEnvironment();

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

@@ -594,7 +594,7 @@ private boolean checkClosed()
                 if (_allocationThread != Thread.currentThread())
                     throw new IllegalStateException("Using request closed by a different thread");
                 else
-                    throw new IllegalStateException("Requset has been closed");
+                    throw new IllegalStateException("Request has been closed");
             }
             return true;
         }