From 60c37a102eee10d267f8ae1777b0ace7623b1986 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 18 Aug 2026 15:27:22 -0700 Subject: [PATCH 1/2] Update TableSelectorTestCase to test more MySQL/MariaDB databases (#7946) ## Rationale Improve testing of MySQL and MariaDB databases ## Changes - Detect MySQL and MariaDB data sources based on product name, not data source name - If `sakila` is not present, try testing with `sys.sys_config` --- .../api/data/TableSelectorTestCase.java | 27 ++++++++++++++++--- .../api/data/dialect/StandardJdbcHelper.java | 2 +- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/data/TableSelectorTestCase.java b/api/src/org/labkey/api/data/TableSelectorTestCase.java index 281681ebc2e..68d5c2ce623 100644 --- a/api/src/org/labkey/api/data/TableSelectorTestCase.java +++ b/api/src/org/labkey/api/data/TableSelectorTestCase.java @@ -17,6 +17,7 @@ import org.apache.commons.lang3.mutable.MutableInt; import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.Logger; import org.junit.Test; import org.labkey.api.collections.CsvSet; import org.labkey.api.data.Selector.ForEachBlock; @@ -30,6 +31,7 @@ import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.TestContext; +import org.labkey.api.util.logging.LogHelper; import org.springframework.jdbc.UncategorizedSQLException; import java.sql.ResultSet; @@ -50,6 +52,8 @@ public class TableSelectorTestCase extends AbstractSelectorTestCase { + private static final Logger LOG = LogHelper.getLogger(TableSelectorTestCase.class, "Test progress"); + @Test public void testTableSelector() throws SQLException { @@ -59,13 +63,25 @@ public void testTableSelector() throws SQLException // testTableSelector(DbSchema.get("oracle.granite", DbSchemaType.Bare).getTable("account"), Account.class); // Test MySQL or MariaDB database, if present - List mySqlScopes = Stream.of("mySql", "mariadb") - .map(DbScope::getDbScope).filter(Objects::nonNull).toList(); + List mySqlScopes = DbScope.getDbScopesToTest().stream() + .filter(scope -> Set.of("MySQL", "MariaDB").contains(scope.getSqlDialect().getProductName())) + .toList(); + for (DbScope mySqlScope: mySqlScopes) { DbSchema sakila = mySqlScope.getSchema("sakila", DbSchemaType.Bare); if (sakila.existsInDatabase()) - testTableSelector(sakila.getTable("country"), Country.class); + { + testTableSelector(sakila.getTable("Country"), Country.class); + } + else + { + DbSchema sys = mySqlScope.getSchema("sys", DbSchemaType.Bare); + if (sys.existsInDatabase()) + { + testTableSelector(sys.getTable("sys_config"), Config.class); + } + } } testTableSelector(CoreSchema.getInstance().getTableInfoActiveUsers(), User.class); testTableSelector(CoreSchema.getInstance().getTableInfoModules(), ModuleContext.class); @@ -124,6 +140,8 @@ public int hashCode() } } + record Config(String Variable, String Value, Date Set_Time, String Set_By){} + // public static class Account // { // private int _account_id; @@ -430,6 +448,9 @@ private void testColumnList(TableSelector selector, boolean stable) throws SQLEx private void testTableSelector(TableInfo table, Class clazz) throws SQLException { + DbSchema schema = table.getSchema(); + LOG.info("Testing {}.{}.{}", schema.getScope().getDisplayName(), schema.getName(), table.getName()); + TableSelector selector = new TableSelector(table); test(selector, clazz); diff --git a/api/src/org/labkey/api/data/dialect/StandardJdbcHelper.java b/api/src/org/labkey/api/data/dialect/StandardJdbcHelper.java index 986b0e0fa97..9362ddcde91 100644 --- a/api/src/org/labkey/api/data/dialect/StandardJdbcHelper.java +++ b/api/src/org/labkey/api/data/dialect/StandardJdbcHelper.java @@ -44,7 +44,7 @@ protected String parseDatabase(String url) throws ServletException if (-1 == dbEnd) dbEnd = url.length(); - // Last '/' is the database delimiter, except for "jdbc:postgresql:database" + // Last '/' is the database delimiter, except for "jdbc:postgresql:database" and old Oracle formats char dbDelimiter = url.contains("/") ? '/' : ':'; int dbDelimiterIndex = url.lastIndexOf(dbDelimiter, dbEnd); From 6881a5b9e9e744dde4e35452b328c98d9de5e8ae Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Fri, 21 Aug 2026 14:15:39 -0700 Subject: [PATCH 2/2] Merge 25.11->26.8 (#7966) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Rationale Merge 25.11->26.8 ## Related Pull Requests - ## Changes - --- core/src/org/labkey/core/CoreModule.java | 1 + .../labkey/core/admin/AdminController.java | 129 ++++++++++++++++-- 2 files changed, 117 insertions(+), 13 deletions(-) diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 9a7ef72f8b0..469ca1149f4 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -1508,6 +1508,7 @@ public TabDisplayMode getTabDisplayMode() public @NotNull Set> getUnitTests() { return Set.of( + AdminController.FileRootPermissionTestCase.class, ApiJsonWriter.TestCase.class, ClassLoaderTestCase.class, CopyFileRootPipelineJob.TestCase.class, diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index aada2d04fd8..874fef79b50 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -6433,6 +6433,7 @@ else if (form.hasSiteDefaultRoot()) { if (service.isFileRootDisabled(ctx.getContainer()) || !service.isUseDefaultRoot(ctx.getContainer())) { + throwIfUnauthorizedFileRootChange(ctx, service, form); service.setIsUseDefaultRoot(ctx.getContainer(), true); changed = true; shouldCopyMove = true; @@ -6570,22 +6571,62 @@ private static void initiateCopyFilesPipelineJobs(ViewContext ctx, @NotNull List private static void throwIfUnauthorizedFileRootChange(ViewContext ctx, FileContentService service, FileManagementForm form) { - // test permissions. only site admins are able to turn on a custom file root for a folder - // this is only relevant if the folder is either being switched to a custom file root, - // or if the file root is changed. - if (!service.isUseDefaultRoot(ctx.getContainer())) - { - Path fileRootPath = service.getFileRootPath(ctx.getContainer()); - if (null != fileRootPath) + // Only site admins (AdminOperationsPermission) are able to switch a folder to a custom file root, change + // an existing custom root's path, or revert a custom root back to the site default -- any of these moves + // where the folder's files live. Resubmitting the folder's own current root unchanged is a no-op and does + // not require the elevated permission. + boolean hasAdminOpsPermission = ctx.getUser().hasRootPermission(AdminOperationsPermission.class); + boolean isUseDefaultRoot = service.isUseDefaultRoot(ctx.getContainer()); + String requestedRoot; + String currentRoot; + + if (form.hasSiteDefaultRoot()) + { + // Requesting the default root: no root is being submitted; the current custom root (if any) is + // whatever type the container has now. + requestedRoot = null; + if (isUseDefaultRoot) + currentRoot = null; + else if (service.isCloudRoot(ctx.getContainer())) + currentRoot = service.getCloudRootName(ctx.getContainer()); + else { - String absolutePath = FileUtil.getAbsolutePath(ctx.getContainer(), fileRootPath); - if (Strings.CI.equals(absolutePath, form.getFolderRootPath())) - { - if (!ctx.getUser().hasRootPermission(AdminOperationsPermission.class)) - throw new UnauthorizedException("Only site admins can change file roots"); - } + Path fileRootPath = service.getFileRootPath(ctx.getContainer()); + currentRoot = null != fileRootPath ? FileUtil.getAbsolutePath(ctx.getContainer(), fileRootPath) : null; } } + else if (form.isCloudFileRoot()) + { + requestedRoot = form.getCloudRootName(); + currentRoot = (!isUseDefaultRoot && service.isCloudRoot(ctx.getContainer())) ? service.getCloudRootName(ctx.getContainer()) : null; + } + else + { + requestedRoot = StringUtils.trimToNull(form.getFolderRootPath()); + Path fileRootPath = isUseDefaultRoot ? null : service.getFileRootPath(ctx.getContainer()); + currentRoot = null != fileRootPath ? FileUtil.getAbsolutePath(ctx.getContainer(), fileRootPath) : null; + } + + if (!isFileRootChangeAuthorizedOrNoChange(hasAdminOpsPermission, isUseDefaultRoot, currentRoot, requestedRoot)) + throw new UnauthorizedException("Only site admins can change file roots"); + } + + /** + * Pure decision logic behind {@link #throwIfUnauthorizedFileRootChange}, factored out for unit testing. + * @param hasAdminOpsPermission whether the requesting user holds root AdminOperationsPermission + * @param isUseDefaultRoot whether the target container currently uses the default (inherited) file root + * @param currentRoot the container's existing custom root path/cloud name, or null if there isn't one + * @param requestedRoot the root path/cloud name submitted in the request, or null if none was submitted + */ + private static boolean isFileRootChangeAuthorizedOrNoChange(boolean hasAdminOpsPermission, boolean isUseDefaultRoot, @Nullable String currentRoot, @Nullable String requestedRoot) + { + if (hasAdminOpsPermission) + return true; + if (null == requestedRoot) + return isUseDefaultRoot || null == currentRoot; // clearing an existing custom root is still a change to it + if (!isUseDefaultRoot && requestedRoot.equalsIgnoreCase(currentRoot)) + return true; // no-op resubmission of the folder's own existing custom root + return false; } public static void setEnabledCloudStores(ViewContext ctx, FileManagementForm form, BindException errors) @@ -9564,6 +9605,68 @@ public void modulesWithSchemaVersionButNoScripts() } } + // Regression coverage for the file root privilege-escalation fix: a folder/project admin without root + // AdminOperationsPermission must never be able to switch a container to a custom file root, or change an + // existing custom root's path. + public static class FileRootPermissionTestCase extends Assert + { + @Test + public void defaultRootRequiresAdminOpsPermissionForNewCustomPath() + { + // This is the case the original (inverted) condition silently skipped: a container on the default + // root, submitting any custom path, from a user without AdminOperationsPermission. + assertFalse(isFileRootChangeAuthorizedOrNoChange(false, true, null, "/some/path")); + } + + @Test + public void customRootRequiresAdminOpsPermissionForDifferentPath() + { + assertFalse(isFileRootChangeAuthorizedOrNoChange(false, false, "/existing/path", "/attacker/path")); + } + + @Test + public void customRootResubmissionOfSamePathIsAllowed() + { + assertTrue(isFileRootChangeAuthorizedOrNoChange(false, false, "/existing/path", "/existing/path")); + assertTrue(isFileRootChangeAuthorizedOrNoChange(false, false, "/Existing/Path", "/existing/path")); + } + + @Test + public void noRequestedRootOnDefaultRootIsAllowed() + { + // No custom root requested and none currently exists -- nothing to protect. + assertTrue(isFileRootChangeAuthorizedOrNoChange(false, true, null, null)); + } + + @Test + public void clearingAnExistingCustomRootRequiresAdminOpsPermission() + { + // A request that omits the root (e.g. a non-ops admin's disabled form fields not being submitted) + // must not be able to silently clear an existing custom root back to default. + assertFalse(isFileRootChangeAuthorizedOrNoChange(false, false, "/existing/path", null)); + assertTrue(isFileRootChangeAuthorizedOrNoChange(true, false, "/existing/path", null)); + } + + @Test + public void revertingCustomRootToSiteDefaultRequiresAdminOpsPermission() + { + // Selecting the site default root while a custom root (file path or cloud) is in effect relocates the + // folder's file storage, so it needs the same elevated permission as setting a custom root. + assertFalse(isFileRootChangeAuthorizedOrNoChange(false, false, "/existing/path", null)); + assertFalse(isFileRootChangeAuthorizedOrNoChange(false, false, "myCloudStore", null)); + assertTrue(isFileRootChangeAuthorizedOrNoChange(true, false, "myCloudStore", null)); + // Already on the default root (e.g. re-enabling file sharing from the disabled state) is a no-op. + assertTrue(isFileRootChangeAuthorizedOrNoChange(false, true, null, null)); + } + + @Test + public void adminOpsPermissionIsAlwaysAllowed() + { + assertTrue(isFileRootChangeAuthorizedOrNoChange(true, true, null, "/some/path")); + assertTrue(isFileRootChangeAuthorizedOrNoChange(true, false, "/existing/path", "/attacker/path")); + } + } + public static class ModuleForm { private String _name;