diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 83170e3529d..1a0ebb21c07 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -189,6 +189,7 @@ import org.labkey.api.view.ViewServlet; import org.labkey.api.view.WebPartFactory; import org.labkey.api.webdav.WebdavResolverImpl; +import org.labkey.api.wiki.WikiRendererType; import org.labkey.api.writer.ContainerUser; import org.labkey.filters.ContentSecurityPolicyFilter; @@ -562,6 +563,7 @@ public void registerServlets(ServletContext servletCtx) UserManager.TestCase.class, ViewCategoryManager.TestCase.class, WebdavResolverImpl.TestCase.class, + WikiRendererType.TestCase.class, WorkbookContainerType.TestCase.class, WriteableLookAndFeelProperties.TestCase.class ); diff --git a/api/src/org/labkey/api/admin/AdminBean.java b/api/src/org/labkey/api/admin/AdminBean.java index f61e1d45f07..4c655762208 100644 --- a/api/src/org/labkey/api/admin/AdminBean.java +++ b/api/src/org/labkey/api/admin/AdminBean.java @@ -24,7 +24,6 @@ import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CoreSchema; import org.labkey.api.data.DbScope; -import org.labkey.api.module.Module; import org.labkey.api.module.ModuleLoader; import org.labkey.api.security.UserManager; import org.labkey.api.settings.AppProps; @@ -43,7 +42,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -82,7 +80,6 @@ public static class RecentUser public static final String sessionTimeout = Formats.commaf0.format(ModuleLoader.getServletContext().getSessionTimeout()); public static final String buildTime = ModuleLoader.getInstance().getCoreModule().getBuildTime(); public static final String serverStartupTime = DateUtil.formatDateTime(ContainerManager.getRoot()); - public static final List modules; public static String asserts = "disabled"; @@ -116,9 +113,6 @@ public static class RecentUser //noinspection ConstantConditions,AssertWithSideEffects assert null != (asserts = "enabled"); - - modules = new ArrayList<>(ModuleLoader.getInstance().getModules()); - modules.sort(Comparator.comparing(Module::getName, String.CASE_INSENSITIVE_ORDER)); } private static @Nullable String getValue(Field field) diff --git a/api/src/org/labkey/api/data/DataColumn.java b/api/src/org/labkey/api/data/DataColumn.java index 7b66f384a75..0f90f9b2012 100644 --- a/api/src/org/labkey/api/data/DataColumn.java +++ b/api/src/org/labkey/api/data/DataColumn.java @@ -191,6 +191,12 @@ protected ColumnInfo getDisplayField(@NotNull ColumnInfo col, boolean withLookup return null==display ? col : display; } + @Override + public void setWithLookup(boolean withLookup) + { + _displayColumn = withLookup ? getDisplayField(_boundColumn, true) : _boundColumn; + } + @Override public String toString() { diff --git a/api/src/org/labkey/api/data/DisplayColumn.java b/api/src/org/labkey/api/data/DisplayColumn.java index a954198c9c1..659a1b69dda 100644 --- a/api/src/org/labkey/api/data/DisplayColumn.java +++ b/api/src/org/labkey/api/data/DisplayColumn.java @@ -1232,6 +1232,11 @@ public void setRequiresHtmlFiltering(boolean requiresHtmlFiltering) _requiresHtmlFiltering = requiresHtmlFiltering; } + public void setWithLookup(boolean withLookup) + { + // subclasses override as needed + } + public void setLinkTarget(String linkTarget) { _linkTarget = linkTarget; diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index 1b74eec0fa7..fa53d3cf3c9 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -924,6 +924,19 @@ public boolean supportsNativeGreatestAndLeast() return true; } + @Override + public boolean supportsIsNumeric() + { + return true; + } + + @Override + public SQLFragment isNumericExpr(SQLFragment expression) + { + return new SQLFragment("(CASE WHEN CAST((").append(expression) + .append(") AS TEXT) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)$' THEN 1 ELSE 0 END)"); + } + private class PostgreSqlColumnMetaDataReader extends ColumnMetaDataReader { private final TableInfo _table; diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index 06f31efb50b..73cdd4cadb4 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SqlDialect.java @@ -59,6 +59,7 @@ import org.labkey.api.module.ModuleLoader; import org.labkey.api.query.FieldKey; import org.labkey.api.util.ExceptionUtil; +import org.labkey.api.util.HtmlString; import org.labkey.api.util.MemTracker; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.SystemMaintenance; @@ -80,6 +81,8 @@ import java.sql.SQLException; import java.sql.Statement; import java.sql.Types; +import java.time.Duration; +import java.time.LocalDateTime; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -95,6 +98,7 @@ import java.util.regex.Pattern; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -168,7 +172,7 @@ public String getOtherDatabaseThreads() { StringBuilder sb = new StringBuilder(); - // Per 18789, also include threads without db connections. + // Per Issue 18789, also include threads without db connections. List dbThreads = new ArrayList<>(); for (Map.Entry entry : Thread.getAllStackTraces().entrySet()) @@ -868,6 +872,16 @@ public SQLFragment getGreatestAndLeastSQL(String method, SQLFragment... argument throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement"); } + public boolean supportsIsNumeric() + { + return false; + } + + public SQLFragment isNumericExpr(SQLFragment expression) + { + throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement"); + } + public void handleCreateDatabaseException(SQLException e) throws ServletException { throw(new ServletException("Can't create database", e)); @@ -1980,12 +1994,61 @@ public final Collection getExecutionPlan(DbScope scope, SQLFragment sql, // Add any database configuration warnings (e.g., missing aggregate function or deprecated database server version) // to display in the page header for administrators. This will be called: // - Only on the LabKey DataSource's dialect instance (not external data sources) - // - After the core module has been upgraded and the dialect has been prepared for the last time, meaning the dialect + // - After the core module has been upgraded, and the dialect has been prepared for the last time, meaning the dialect // should reflect the final database configuration public void addAdminWarningMessages(Warnings warnings, boolean showAllWarnings) { } + public static final long TIME_DIFFERENCE_WARNING_SECONDS = 10; + + public static ServerDatabaseTimeDifference getServerDatabaseTimeDifference(DbScope scope) + { + // Compare LocalDateTime to capture any difference in server and database times (skew or timezone). + LocalDateTime serverTime = LocalDateTime.now(); + LocalDateTime databaseTime = new SqlSelector(scope, "SELECT CURRENT_TIMESTAMP").getObject(LocalDateTime.class); + + return new ServerDatabaseTimeDifference(serverTime, databaseTime); + } + + public record ServerDatabaseTimeDifference(LocalDateTime serverTime, LocalDateTime databaseTime) + { + public long getSeconds() + { + return Math.abs(Duration.between(serverTime, databaseTime).toSeconds()); + } + + public boolean exceedsWarningThreshold() + { + return getSeconds() > TIME_DIFFERENCE_WARNING_SECONDS; + } + } + + // GH Issue #1223: Add a site configuration warning for administrators if the server and database clocks differ. + protected void addTimeDifferenceWarning(Warnings warnings, boolean showAllWarnings) + { + try + { + ServerDatabaseTimeDifference difference = getServerDatabaseTimeDifference(DbScope.getLabKeyScope()); + + if (difference.exceedsWarningThreshold()) + warnings.add(getTimeDifferenceWarning(difference.getSeconds())); + else if (showAllWarnings) + warnings.add(getTimeDifferenceWarning(TIME_DIFFERENCE_WARNING_SECONDS + 1)); + } + catch (Exception e) + { + LOG.warn("Unable to compare web server and database server times", e); + } + } + + private HtmlString getTimeDifferenceWarning(long seconds) + { + return HtmlString.of("The web server and database server times differ by " + seconds + " seconds. " + + "LabKey Server often relies on comparing timestamps stored in the database with timestamps generated by " + + "the web server, so this difference can lead to data integrity issues. Synchronize the clocks on these servers."); + } + public abstract List getChangeStatements(TableChange change); public abstract void purgeTempSchema(Map createdTableNames); @@ -2007,7 +2070,7 @@ protected void trackTempTables(Map createdTableNames) // Defragment an index, if necessary public void defragmentIndex(DbSchema schema, String tableSelectName, String indexName) { - // By default do nothing + // By default, do nothing } public boolean isTableExists(DbScope scope, String schema, String name) @@ -2035,7 +2098,7 @@ public boolean hasTriggers(DbSchema scope, String schema, String tableName) /** * - * @return true If the dialect is one supported for the backend LabKey database. ie, Postgres or SQL Server + * @return true If the dialect is one supported for the backend LabKey database. i.e., Postgres or SQL Server */ public boolean isLabKeyDbDialect() { @@ -2409,5 +2472,37 @@ public void testProcedureIdentifierQuoting() // A dotted name is rejected: schema and procedure are separate parameters, so a '.' in a single component is an ambiguous schema-qualified reference assertThrows(IllegalArgumentException.class, () -> dialect.buildProcedureCall(core.getName(), "a.b", 0, false, false, scope)); } + + // GH Issue #1223: Verify the server/database clock-skew arithmetic and warning threshold + @Test + public void testServerDatabaseTimeDifference() + { + LocalDateTime base = LocalDateTime.of(2026, 7, 6, 12, 0, 0); + + // Identical times: zero difference, no warning + ServerDatabaseTimeDifference equal = new ServerDatabaseTimeDifference(base, base); + assertEquals(0, equal.getSeconds()); + assertFalse(equal.exceedsWarningThreshold()); + + // Comfortably below the threshold: no warning + ServerDatabaseTimeDifference below = new ServerDatabaseTimeDifference(base, base.plusSeconds(5)); + assertEquals(5, below.getSeconds()); + assertFalse(below.exceedsWarningThreshold()); + + // Exactly at the threshold: no warning (comparison is strictly greater-than) + ServerDatabaseTimeDifference atThreshold = new ServerDatabaseTimeDifference(base, base.plusSeconds(TIME_DIFFERENCE_WARNING_SECONDS)); + assertEquals(TIME_DIFFERENCE_WARNING_SECONDS, atThreshold.getSeconds()); + assertFalse(atThreshold.exceedsWarningThreshold()); + + // Just past the threshold: warning + ServerDatabaseTimeDifference over = new ServerDatabaseTimeDifference(base, base.plusSeconds(TIME_DIFFERENCE_WARNING_SECONDS + 1)); + assertEquals(TIME_DIFFERENCE_WARNING_SECONDS + 1, over.getSeconds()); + assertTrue(over.exceedsWarningThreshold()); + + // Difference is absolute: database clock behind the web server is treated the same as ahead + ServerDatabaseTimeDifference behind = new ServerDatabaseTimeDifference(base, base.minusSeconds(TIME_DIFFERENCE_WARNING_SECONDS + 1)); + assertEquals(TIME_DIFFERENCE_WARNING_SECONDS + 1, behind.getSeconds()); + assertTrue(behind.exceedsWarningThreshold()); + } } } diff --git a/api/src/org/labkey/api/mcp/AbstractAgentAction.java b/api/src/org/labkey/api/mcp/AbstractAgentAction.java index 0affd4c5517..a8bb158aab1 100644 --- a/api/src/org/labkey/api/mcp/AbstractAgentAction.java +++ b/api/src/org/labkey/api/mcp/AbstractAgentAction.java @@ -85,9 +85,18 @@ protected String handleEscape(String prompt) @Override public void validateForm(F form, Errors errors) { + if (!McpService.get().isAIFeaturesEnabled()) + { + errors.reject(ERROR_GENERIC, "Agent actions are not available."); + return; + } + String prompt = form.getPrompt(); if (prompt != null && prompt.length() > MAX_PROMPT_CHAR_LENGTH) + { errors.rejectValue("prompt", ERROR_GENERIC, "Prompt cannot exceed " + MAX_PROMPT_CHAR_LENGTH + " characters."); + return; + } // Only honor a client-supplied conversationId if this agent previously issued this session that // id. Otherwise, generate a fresh one. This prevents a same-session caller from splicing into diff --git a/api/src/org/labkey/api/mcp/McpService.java b/api/src/org/labkey/api/mcp/McpService.java index 3841c9ae145..ff1b009feac 100644 --- a/api/src/org/labkey/api/mcp/McpService.java +++ b/api/src/org/labkey/api/mcp/McpService.java @@ -25,6 +25,8 @@ import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.util.HtmlString; import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.HtmlView; +import org.labkey.api.view.HttpView; import org.labkey.api.writer.ContainerUser; import org.springframework.ai.chat.client.ChatClient; import org.springframework.ai.chat.model.ToolContext; @@ -36,6 +38,7 @@ import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.function.Supplier; /// @@ -94,8 +97,11 @@ /// public interface McpService extends ToolCallbackProvider { + String VECTOR_SCHEMA = "vector_indexes"; // for pgvector extension, see core-26.005-26.006.sql + Logger LOG = LogHelper.getLogger(McpService.class, "MCP registration exceptions"); String ENABLE_MCP_SERVER_FLAG = "enableMcpServer"; + String ENABLE_AI_FEATURES = "enableAIFeatures"; // Interface for MCP classes that we will "ingest" using Spring annotations. Provides a few helper methods. interface McpImpl @@ -117,9 +123,6 @@ default User getUser(ToolContext toolContext) // Every MCP resource should call this on every invocation default void incrementResourceRequestCount(String resource) { - if (!get().isEnabled()) - throw new RuntimeException("The MCP server is not enabled for external requests. Consider toggling the optional feature flag."); - get().incrementResourceRequestCount(resource); } } @@ -142,8 +145,19 @@ default boolean isEnabled() return OptionalFeatureService.get().isFeatureEnabled(ENABLE_MCP_SERVER_FLAG); } + default boolean isAIFeaturesEnabled() + { + return OptionalFeatureService.get().isFeatureEnabled(ENABLE_AI_FEATURES); + } + boolean isReady(); + // Convenience for in-product AI features: the AI feature flag is on AND the service has started. + default boolean isAIFeaturesReady() + { + return isAIFeaturesEnabled() && isReady(); + } + // Register MCPs in Module.startup() default void register(McpImpl mcp) { @@ -187,6 +201,8 @@ default ChatClient getChat(HttpSession session, String conversationName, Supplie record MessageResponse(String contentType, String text, HtmlString html) {} + record VectorDocument(String id, String text, Map metadata) {} + /** get a consolidated response (good for many text-oriented agents/use-cases) */ MessageResponse sendMessage(ChatClient chat, String message); @@ -201,4 +217,26 @@ default List sendMessageEx(ChatClient chat, String message) * CONSIDER: Is it possible to implement VectorStoreRetriever wrapper for SearchService??? */ VectorStore getVectorStore(); + + /** Returns true if the vector store exists and contains at least one document. */ + boolean isVectorStorePopulated(@NotNull VectorStore vs); + + /** + * Adds documents to the vector store, automatically splitting any document whose token + * count exceeds the embedding model's input limit. Prefer this over + * {@code getVectorStore().add(...)} for indexing — it prevents the + * {@code IllegalArgumentException} that {@code TokenCountBatchingStrategy} throws on + * oversized inputs. + */ + void addDocuments(List documents); + + void saveVectorStore(); + + /** Drop and recreate the vector store table. Use when the embedding model has changed and dimensions no longer match. */ + void resetVectorStore(); + + default HttpView getAssistantStatusView() + { + return new HtmlView(HtmlString.of("AI Assistant features are not available.")); + } } diff --git a/api/src/org/labkey/api/mcp/NoopMcpService.java b/api/src/org/labkey/api/mcp/NoopMcpService.java index e33eb422a4c..1923c770ccd 100644 --- a/api/src/org/labkey/api/mcp/NoopMcpService.java +++ b/api/src/org/labkey/api/mcp/NoopMcpService.java @@ -42,6 +42,12 @@ public boolean isEnabled() return false; } + @Override + public boolean isAIFeaturesEnabled() + { + return false; + } + @Override public boolean isReady() { @@ -101,4 +107,25 @@ public VectorStore getVectorStore() { return null; } + + @Override + public boolean isVectorStorePopulated(@NotNull VectorStore vs) + { + return false; + } + + @Override + public void addDocuments(List documents) + { + } + + @Override + public void saveVectorStore() + { + } + + @Override + public void resetVectorStore() + { + } } diff --git a/api/src/org/labkey/api/moduleeditor/api/ModuleEditorService.java b/api/src/org/labkey/api/moduleeditor/api/ModuleEditorService.java index cdeb61c0d6f..21b01c10120 100644 --- a/api/src/org/labkey/api/moduleeditor/api/ModuleEditorService.java +++ b/api/src/org/labkey/api/moduleeditor/api/ModuleEditorService.java @@ -81,4 +81,11 @@ default File getFileForModuleResource(Module module, Path path) return null; return FileUtil.appendPath(resources, path); } + + // used by ModuleResourceProvider@AllModuleResourcesRoot() (_webdav/@modules) which wants to soo + // the whole module source directory (e.g. when using SourcePath) + default File getModuleRoot(Module module, @Nullable List messages) + { + return null; + } } \ No newline at end of file diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index dd1133b54bc..eef0416412b 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -892,6 +892,8 @@ public List> updateRows(User user, Container container, List } // Fire triggers, if any, and also throw if there are any errors + if (errors.hasErrors()) + throw errors; getQueryTable().fireBatchTrigger(container, user, TableInfo.TriggerType.UPDATE, null, false, errors, extraScriptContext); afterInsertUpdate(null==result?0:result.size(), errors, true); diff --git a/api/src/org/labkey/api/security/ApiKeyManager.java b/api/src/org/labkey/api/security/ApiKeyManager.java index 69684001985..b4b6cace88d 100644 --- a/api/src/org/labkey/api/security/ApiKeyManager.java +++ b/api/src/org/labkey/api/security/ApiKeyManager.java @@ -22,6 +22,7 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; +import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CoreSchema; import org.labkey.api.data.DbScope; import org.labkey.api.data.DbScope.Transaction; @@ -39,6 +40,15 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.security.UserManager.SessionHandler; import org.labkey.api.security.ValidEmail.InvalidEmailException; +import org.labkey.api.security.permissions.AdminPermission; +import org.labkey.api.security.permissions.DeletePermission; +import org.labkey.api.security.permissions.InsertPermission; +import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.security.permissions.UpdatePermission; +import org.labkey.api.security.roles.EditorRole; +import org.labkey.api.security.roles.ReaderRole; +import org.labkey.api.security.roles.Role; +import org.labkey.api.security.roles.RoleManager; import org.labkey.api.settings.AppProps; import org.labkey.api.settings.LenientStartupPropertyHandler; import org.labkey.api.settings.StartupProperty; @@ -60,6 +70,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Stream; import static org.labkey.api.util.IntegerUtils.asInteger; @@ -85,9 +96,9 @@ private ApiKeyManager() * @param user User to be associated with the new API key. * @return An API key that expires after the admin-configured duration */ - public @NotNull String createKey(@NotNull User user, @Nullable String description) + public @NotNull String createKey(@NotNull User user, @Nullable String description, @Nullable Class restrictionRole) { - return createKey(user, AppProps.getInstance().getApiKeyExpirationSeconds(), description); + return createKey(user, AppProps.getInstance().getApiKeyExpirationSeconds(), description, restrictionRole); } /** @@ -97,6 +108,18 @@ private ApiKeyManager() * @return An API key that expires after the specified number of seconds */ public @NotNull String createKey(@NotNull User user, int expirationSeconds, @Nullable String description) + { + return createKey(user, expirationSeconds, description, null); + } + + /** + * Create an API key associated with a user and persist it in the database. + * @param user User to be associated with the new API key. + * @param expirationSeconds Number of seconds until expiration. -1 means no expiration. + * @param restrictionRole Role class that limits this API key's permissions. null means no restrictions. + * @return An API key that expires after the specified number of seconds + */ + public @NotNull String createKey(@NotNull User user, int expirationSeconds, @Nullable String description, @Nullable Class restrictionRole) { if (user.isGuest()) throw new IllegalStateException("Can't create an API key for a guest"); @@ -120,6 +143,9 @@ private ApiKeyManager() if (description != null) map.put("Description", StringUtils.abbreviate(description.trim(), 256)); + if (restrictionRole != null) + map.put("RestrictionRole", restrictionRole.getName()); + try (Transaction t = CoreSchema.getInstance().getScope().beginTransaction(TRANSACTION_KIND)) { Table.insert(user, CoreSchema.getInstance().getTableAPIKeys(), map); @@ -183,17 +209,35 @@ public void updateLastUsed(String apikey) try (Transaction t = scope.beginTransaction(TRANSACTION_KIND)) { - SQLFragment sql = new SQLFragment("UPDATE " + CoreSchema.getInstance().getTableAPIKeys() + " SET LastUsed = ? WHERE Crypt = ?", new Date(), crypt(apikey)); + SQLFragment sql = new SQLFragment("UPDATE ") + .append(CoreSchema.getInstance().getTableAPIKeys()) + .append(" SET LastUsed = ? WHERE Crypt = ?") + .add(new Date()) + .add(crypt(apikey)); new SqlExecutor(scope).execute(sql); t.commit(); } } - public record ApiKeyAuthentication(int createdBy, int rowId) + public record ApiKeyAuthentication(int createdBy, int rowId, @Nullable Class restrictionRole) { - public User getUser() + public @Nullable User getUser() { - return UserManager.getUser(createdBy()); + User user = UserManager.getUser(createdBy()); + if (restrictionRole != null) + { + Role role = RoleManager.getRole(restrictionRole); + if (role == null) + { + LOG.error("API key for {} specifies a restriction role {} that was not found", user, restrictionRole.getName()); + user = null; + } + else + { + user = new PermissionsRestrictedUser(user, role.getPermissions()); + } + } + return user; } } @@ -212,7 +256,7 @@ public User getUser() try (Transaction t = CoreSchema.getInstance().getScope().beginTransaction(TRANSACTION_KIND)) { - ret = new TableSelector(CoreSchema.getInstance().getTableAPIKeys(), Set.of("CreatedBy", "RowId"), filter, null).getObject(ApiKeyAuthentication.class); + ret = new TableSelector(CoreSchema.getInstance().getTableAPIKeys(), Set.of("CreatedBy", "RowId", "RestrictionRole"), filter, null).getObject(ApiKeyAuthentication.class); t.commit(); } @@ -327,6 +371,53 @@ public void testTransaction() ApiKeyManager.get().deleteKey(apikey); assertNull(ApiKeyManager.get().authenticateFromApiKey(apikey)); } + + private record UserAndKey(User user, String apiKey){} + + @Test + public void testRoleRestrictions() + { + User admin = TestContext.get().getUser(); + UserAndKey readerUAK = createApiKeyAndRetrieveUser(admin, ReaderRole.class); + User reader = readerUAK.user(); + UserAndKey editorUAK = createApiKeyAndRetrieveUser(admin, EditorRole.class); + User editor = editorUAK.user(); + + ContainerManager.getAllChildren(ContainerManager.getRoot(), admin, AdminPermission.class).stream() + .limit(5) + .forEach(child -> { + assertTrue(child.hasPermission(admin, AdminPermission.class)); + assertFalse(child.hasPermission(editor, AdminPermission.class)); + assertFalse(child.hasPermission(reader, AdminPermission.class)); + Stream.of(DeletePermission.class, UpdatePermission.class, InsertPermission.class) + .forEach(perm -> { + assertTrue(child.hasPermission(admin, perm)); + assertTrue(child.hasPermission(editor, perm)); + assertFalse(child.hasPermission(reader, perm)); + }); + assertTrue(child.hasPermission(admin, ReadPermission.class)); + assertTrue(child.hasPermission(editor, ReadPermission.class)); + assertTrue(child.hasPermission(reader, ReadPermission.class)); + }); + + ApiKeyManager.get().deleteKey(readerUAK.apiKey()); + ApiKeyManager.get().deleteKey(editorUAK.apiKey()); + assertNull(ApiKeyManager.get().authenticateFromApiKey(readerUAK.apiKey())); + assertNull(ApiKeyManager.get().authenticateFromApiKey(editorUAK.apiKey())); + } + + private UserAndKey createApiKeyAndRetrieveUser(User user, Class restrictionRole) + { + String apiKey = ApiKeyManager.get().createKey(user, 10, "Created by ApiKeyManager.TestCase", restrictionRole); + ApiKeyAuthentication auth = ApiKeyManager.get().authenticateFromApiKey(apiKey); + assertNotNull(auth); + User restrictedUser = auth.getUser(); + assertNotNull(restrictedUser); + assertEquals(user.getUserId(), restrictedUser.getUserId()); + assertTrue(restrictedUser instanceof PermissionsRestrictedUser); + + return new UserAndKey(restrictedUser, apiKey); + } } public static class ApiKeyMaintenanceTask implements MaintenanceTask diff --git a/api/src/org/labkey/api/security/AuthFilter.java b/api/src/org/labkey/api/security/AuthFilter.java index 21e2eb90061..0badfd517c0 100644 --- a/api/src/org/labkey/api/security/AuthFilter.java +++ b/api/src/org/labkey/api/security/AuthFilter.java @@ -55,7 +55,6 @@ public class AuthFilter implements Filter private static final Object FIRST_REQUEST_LOCK = new Object(); public static final String STRICT_TRANSPORT_SECURITY_HEADER_NAME = "Strict-Transport-Security"; - public static final String X_FRAME_OPTIONS_HEADER_NAME = "X-Frame-Options"; public static final String X_CONTENT_TYPE_OPTIONS_HEADER_NAME = "X-Content-Type-Options"; public static final String REFERRER_POLICY_HEADER_NAME = "Referrer-Policy"; public static final String SERVER_HEADER_NAME = "Server"; @@ -87,8 +86,6 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha if (ModuleLoader.getInstance().isStartupComplete()) { - if (!"ALLOW".equals(AppProps.getInstance().getXFrameOption())) - resp.setHeader(X_FRAME_OPTIONS_HEADER_NAME, AppProps.getInstance().getXFrameOption()); resp.setHeader(X_CONTENT_TYPE_OPTIONS_HEADER_NAME, "nosniff"); resp.setHeader(REFERRER_POLICY_HEADER_NAME, "origin-when-cross-origin" ); diff --git a/api/src/org/labkey/api/security/ClonedUser.java b/api/src/org/labkey/api/security/ClonedUser.java index b5870e4bf36..591d3511db3 100644 --- a/api/src/org/labkey/api/security/ClonedUser.java +++ b/api/src/org/labkey/api/security/ClonedUser.java @@ -45,7 +45,7 @@ protected ClonedUser(String email, int userId, String displayName, String firstN setPhone(phone); setLastActivity(lastActivity); - setImpersonationContext(ctx); + setPermissionsContext(ctx); } // Map a stream of role classes to a set of roles diff --git a/api/src/org/labkey/api/security/ElevatedUser.java b/api/src/org/labkey/api/security/ElevatedUser.java index c65bd223d75..88915061029 100644 --- a/api/src/org/labkey/api/security/ElevatedUser.java +++ b/api/src/org/labkey/api/security/ElevatedUser.java @@ -15,6 +15,7 @@ */ package org.labkey.api.security; +import com.google.common.collect.Streams; import org.labkey.api.audit.permissions.CanSeeAuditLogPermission; import org.labkey.api.data.Container; import org.labkey.api.security.permissions.Permission; @@ -26,6 +27,7 @@ import java.util.Collection; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; /** * A wrapped user that possesses all the security properties (groups, roles, impersonation status, etc.) of the @@ -35,9 +37,26 @@ */ public class ElevatedUser extends ClonedUser { + private static class ElevatedUserContext extends WrappedPermissionsContext + { + private final Set _additionalRoles; + + private ElevatedUserContext(PermissionsContext delegate, Set additionalRoles) + { + super(delegate); + _additionalRoles = additionalRoles; + } + + @Override + public Stream getAssignedRoles(User user, SecurableResource resource) + { + return Streams.concat(_additionalRoles.stream(), super.getAssignedRoles(user, resource)); + } + } + private ElevatedUser(User user, Set rolesToAdd) { - super(user, new WrappedPermissionsContext(user.getPermissionsContext(), rolesToAdd)); + super(user, new ElevatedUserContext(user.getPermissionsContext(), rolesToAdd)); } private ElevatedUser(User user, PermissionsContext ctx) diff --git a/api/src/org/labkey/api/security/LimitedUser.java b/api/src/org/labkey/api/security/LimitedUser.java index cf618ce638c..0febeaf697e 100644 --- a/api/src/org/labkey/api/security/LimitedUser.java +++ b/api/src/org/labkey/api/security/LimitedUser.java @@ -97,7 +97,7 @@ private LimitedUser( @JsonProperty("_lastLogin") Date lastLogin, @JsonProperty("_phone") String phone, @JsonProperty("_lastActivity") Date lastActivity, - @JsonProperty("_impersonationContext") PermissionsContext ctx + @JsonProperty("_permissionsContext") PermissionsContext ctx ) { super(name, userId, displayName, firstName, lastName, active, lastLogin, phone, lastActivity, ctx); diff --git a/api/src/org/labkey/api/security/PermissionsContext.java b/api/src/org/labkey/api/security/PermissionsContext.java index 9c6eeb86095..f4fc81021e9 100644 --- a/api/src/org/labkey/api/security/PermissionsContext.java +++ b/api/src/org/labkey/api/security/PermissionsContext.java @@ -16,6 +16,7 @@ package org.labkey.api.security; import com.google.common.collect.Streams; +import jakarta.servlet.http.HttpSession; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; @@ -87,4 +88,9 @@ default Stream> filterPermissions(Stream> _allowedPermissions; + + private RoleRestrictedPermissionsContext(PermissionsContext ctx, Set> allowedPermissions) + { + super(ctx); + _allowedPermissions = allowedPermissions; + } + + @Override + public Stream> filterPermissions(Stream> perms) + { + return super.filterPermissions(perms).filter(_allowedPermissions::contains); + } + + @Override + public void modifySession(HttpSession session) + { + super.modifySession(session); + session.setAttribute(ALLOWED_PERMISSIONS_KEY, _allowedPermissions); + } + } + + private final @NotNull Set> _allowedPermissions; + + public PermissionsRestrictedUser(User user, @NotNull Set> allowedPermissions) + { + super(user, new RoleRestrictedPermissionsContext(user.getPermissionsContext(), allowedPermissions)); + _allowedPermissions = allowedPermissions; + } + + @Override + public String getPermissionsRestrictions() + { + return "Current user is restricted to these permissions: " + _allowedPermissions; + } +} diff --git a/api/src/org/labkey/api/security/RoleSet.java b/api/src/org/labkey/api/security/RoleSet.java index 58276a200ef..80fac9341b6 100644 --- a/api/src/org/labkey/api/security/RoleSet.java +++ b/api/src/org/labkey/api/security/RoleSet.java @@ -166,7 +166,7 @@ private void testImpersonateRoles(User adminUser, @Nullable Container project, C assertEquals(roles, reconstitutedRoleSet.getRoles()); RoleImpersonationContextFactory factory = new RoleImpersonationContextFactory(project, adminUser, roles, Collections.emptySet(), null); - impersonatingUser.setImpersonationContext(factory.getImpersonationContext()); + impersonatingUser.setPermissionsContext(factory.getImpersonationContext()); if (null == project) assertEquals(roles, impersonatingUser.getSiteRoles(ContainerManager.getRoot()).collect(Collectors.toSet())); diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index 84059487c84..1f9cc19f85a 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -148,6 +148,7 @@ import java.util.stream.Stream; import static org.labkey.api.action.SpringActionController.ERROR_MSG; +import static org.labkey.api.security.PermissionsRestrictedUser.ALLOWED_PERMISSIONS_KEY; import static org.labkey.api.util.IntegerUtils.asInteger; /** @@ -517,8 +518,13 @@ public static User getSessionUser(HandshakeRequest request) if (null != factory) { - sessionUser.setImpersonationContext(factory.getImpersonationContext()); + sessionUser.setPermissionsContext(factory.getImpersonationContext()); } + + //noinspection unchecked + Set> allowedPermissions = (Set>) session.getAttribute(ALLOWED_PERMISSIONS_KEY); + if (allowedPermissions != null) + sessionUser = new PermissionsRestrictedUser(sessionUser, allowedPermissions); } } @@ -586,7 +592,7 @@ public static Pair attemptAuthentication(HttpServletRe if (!sessionUser.isImpersonated() && "true".equalsIgnoreCase(request.getHeader("LabKey-Disallow-Global-Roles"))) { - sessionUser.setImpersonationContext(DisallowPrivilegedRolesContext.get()); + sessionUser.setPermissionsContext(DisallowPrivilegedRolesContext.get()); } HttpSession session = request.getSession(false); @@ -816,6 +822,7 @@ public static HttpSession setAuthenticatedUser(HttpServletRequest request, @Null newSession.setAttribute(PRIMARY_AUTHENTICATION_CONFIGURATION, configuration.getRowId()); newSession.setAttribute(USER_ATTRIBUTES_KEY, response.getUserAttributeMap()); newSession.setAttribute(AUTHENTICATION_PROPERTIES, response.getAuthenticationProperties()); + user.getPermissionsContext().modifySession(newSession); } return newSession; @@ -1121,7 +1128,7 @@ else if (email.getEmailAddress().indexOf("@") > 0) // Issue 25813: if two users are being inserted at the same time through the addUser method above, we can get deadlock on SQLServer so we // actively lock when doing this select to prevent it from promoting a lock up the chain. - SQLFragment select = new SQLFragment("SELECT UserId FROM " + core.getTableInfoUsersData()); + SQLFragment select = new SQLFragment("SELECT UserId FROM ").append(core.getTableInfoUsersData()); if (core.getSchema().getScope().getSqlDialect().isSqlServer()) select.append(" WITH (UPDLOCK)"); select.append(" WHERE DisplayName = ? AND UserId != ?"); @@ -3343,7 +3350,7 @@ public void testReadOnlyImpersonate() throws InvalidEmailException, UserManageme final User testUser = TestContext.get().getUser(); // this user is subsetted to only permit read permissions (see AllowedForReadOnlyUser) User user = addUser(new ValidEmail("impersonate@test.net"), null, false).getUser(); - user.setImpersonationContext(new ReadOnlyPermissionsContext()); + user.setPermissionsContext(new ReadOnlyPermissionsContext()); Container testFolder = null; diff --git a/api/src/org/labkey/api/security/User.java b/api/src/org/labkey/api/security/User.java index 32d8cfbd45d..1965ac892b0 100644 --- a/api/src/org/labkey/api/security/User.java +++ b/api/src/org/labkey/api/security/User.java @@ -33,8 +33,8 @@ import org.labkey.api.security.permissions.AnalystPermission; import org.labkey.api.security.permissions.ApplicationAdminPermission; import org.labkey.api.security.permissions.BrowserDeveloperPermission; -import org.labkey.api.security.permissions.ImpersonatePermission; import org.labkey.api.security.permissions.DeletePermission; +import org.labkey.api.security.permissions.ImpersonatePermission; import org.labkey.api.security.permissions.ImpersonatePrivilegedSiteRolesPermission; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.Permission; @@ -409,7 +409,7 @@ public void setLastActivity(Date lastActivity) _lastActivity = lastActivity; } - void setImpersonationContext(PermissionsContext permissionsContext) + void setPermissionsContext(PermissionsContext permissionsContext) { _permissionsContext = permissionsContext; } @@ -643,4 +643,9 @@ public String toJSONString() { return String.valueOf(getUserId()); } + + public String getPermissionsRestrictions() + { + return "Current user has unrestricted permissions. AI agents should encourage the use of permissions-restricted API keys instead."; + } } diff --git a/api/src/org/labkey/api/security/WrappedPermissionsContext.java b/api/src/org/labkey/api/security/WrappedPermissionsContext.java index a33506a12af..eeffb655a6e 100644 --- a/api/src/org/labkey/api/security/WrappedPermissionsContext.java +++ b/api/src/org/labkey/api/security/WrappedPermissionsContext.java @@ -15,7 +15,7 @@ */ package org.labkey.api.security; -import com.google.common.collect.Streams; +import jakarta.servlet.http.HttpSession; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; import org.labkey.api.security.impersonation.ImpersonationContextFactory; @@ -24,26 +24,18 @@ import org.labkey.api.view.ActionURL; import org.labkey.api.view.NavTree; -import java.util.Set; import java.util.stream.Stream; /** - * Do not use this class directly; use ElevatedUser instead. + * See subclasses ElevatedUser and RoleRestrictedUser */ -public class WrappedPermissionsContext implements PermissionsContext +abstract class WrappedPermissionsContext implements PermissionsContext { private final PermissionsContext _delegate; - private final Set _additionalRoles; - public WrappedPermissionsContext(PermissionsContext delegate, Set additionalRoles) + public WrappedPermissionsContext(PermissionsContext delegate) { _delegate = delegate; - _additionalRoles = additionalRoles; - } - - public WrappedPermissionsContext(PermissionsContext delegate, Role additionalRole) - { - this(delegate, Set.of(additionalRole)); } @Override @@ -86,7 +78,7 @@ public PrincipalArray getGroups(User user) @Override public Stream getAssignedRoles(User user, SecurableResource resource) { - return Streams.concat(_additionalRoles.stream(), _delegate.getAssignedRoles(user, resource)); + return _delegate.getAssignedRoles(user, resource); } @Override @@ -106,4 +98,10 @@ public Stream> filterPermissions(Stream ,