Skip to content

Commit f21d9fd

Browse files
committed
Annotation-based permission checking for tools
1 parent d378c1a commit f21d9fd

5 files changed

Lines changed: 37 additions & 33 deletions

File tree

api/src/org/labkey/api/mcp/McpService.java

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import org.jetbrains.annotations.NotNull;
77
import org.jspecify.annotations.NonNull;
88
import org.labkey.api.data.Container;
9-
import org.labkey.api.module.McpProvider;
109
import org.labkey.api.security.User;
1110
import org.labkey.api.services.ServiceRegistry;
1211
import org.labkey.api.util.HtmlString;
@@ -52,9 +51,9 @@ default User getUser(ToolContext toolContext)
5251
}
5352

5453
// Every MCP resource should call this on every invocation
55-
default void incrementResourceReadCount(String resource)
54+
default void incrementResourceRequestCount(String resource)
5655
{
57-
get().incrementResourceCount(resource);
56+
get().incrementResourceRequestCount(resource);
5857
}
5958
}
6059

@@ -70,27 +69,18 @@ static void setInstance(McpService service)
7069

7170
boolean isReady();
7271

73-
74-
default void register(McpImpl obj)
72+
default void register(McpImpl mcp)
7573
{
76-
ToolCallback[] tools = ToolCallbacks.from(obj);
77-
if (null != tools && tools.length > 0)
78-
registerTools(Arrays.asList(tools));
74+
ToolCallback[] tools = ToolCallbacks.from(mcp);
75+
if (tools.length > 0)
76+
registerTools(Arrays.asList(tools), mcp);
7977

80-
var resources = new SyncMcpResourceProvider(List.of(obj)).getResourceSpecifications();
78+
var resources = new SyncMcpResourceProvider(List.of(mcp)).getResourceSpecifications();
8179
if (null != resources && !resources.isEmpty())
8280
registerResources(resources);
8381
}
8482

85-
86-
default void register(McpProvider mcp)
87-
{
88-
registerTools(mcp.getMcpTools());
89-
registerPrompts(mcp.getMcpPrompts());
90-
registerResources(mcp.getMcpResources());
91-
}
92-
93-
void registerTools(@NotNull List<ToolCallback> tools);
83+
void registerTools(@NotNull List<ToolCallback> tools, McpImpl mcp);
9484

9585
void registerPrompts(@NotNull List<McpServerFeatures.SyncPromptSpecification> prompts);
9686

@@ -106,7 +96,7 @@ default ChatClient getChat(HttpSession session, String agentName, Supplier<Strin
10696

10797
void saveSessionContainer(ToolContext context, Container container);
10898

109-
void incrementResourceCount(String resource);
99+
void incrementResourceRequestCount(String resource);
110100

111101
ChatClient getChat(HttpSession session, String agentName, Supplier<String> systemPromptSupplier, boolean createIfNotExists);
112102

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,11 @@
2121
import java.lang.annotation.Target;
2222

2323
/**
24-
* Indicates that an action does not require any kind of authentication or permission to invoke. Use with extreme
25-
* caution. Typically, actions marked with this annotation will handle their own permission checks in their own code path.
26-
* User: adam
27-
* Date: Dec 22, 2009
28-
*/
29-
public @Retention(java.lang.annotation.RetentionPolicy.RUNTIME) @Target(ElementType.TYPE)
24+
* Indicates that an action class or an MCP tool method does not require any kind of authentication or permission to
25+
* invoke. Use with extreme caution. Typically, actions marked with this annotation will handle their own permission
26+
* checks in their own code path.
27+
*/
28+
public @Retention(java.lang.annotation.RetentionPolicy.RUNTIME) @Target({ElementType.TYPE, ElementType.METHOD})
3029
@interface RequiresNoPermission
3130
{
3231
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@
2222
import java.lang.annotation.Target;
2323

2424
/**
25-
* Specifies the required permission for an action. It does not imply that the user needs to be logged in or otherwise
26-
* authenticated.
25+
* Specifies the required permission for an action class or an MCP tool method. It does not imply that the user needs
26+
* to be logged in or otherwise authenticated.
2727
*/
28-
@Retention(java.lang.annotation.RetentionPolicy.RUNTIME) @Target(ElementType.TYPE)
28+
@Retention(java.lang.annotation.RetentionPolicy.RUNTIME) @Target({ElementType.TYPE, ElementType.METHOD})
2929
public @interface RequiresPermission
3030
{
3131
Class<? extends Permission> value();

core/src/org/labkey/core/CoreMcp.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@
66
import org.labkey.api.data.Container;
77
import org.labkey.api.data.ContainerManager;
88
import org.labkey.api.mcp.McpService;
9+
import org.labkey.api.security.RequiresNoPermission;
10+
import org.labkey.api.security.RequiresPermission;
911
import org.labkey.api.security.User;
1012
import org.labkey.api.security.permissions.ReadPermission;
1113
import org.labkey.api.settings.AppProps;
1214
import org.labkey.api.settings.LookAndFeelProperties;
1315
import org.labkey.api.study.Study;
1416
import org.labkey.api.study.StudyService;
1517
import org.labkey.api.util.HtmlString;
18+
import org.labkey.api.view.UnauthorizedException;
1619
import org.springframework.ai.chat.model.ToolContext;
1720
import org.springframework.ai.tool.annotation.Tool;
1821
import org.springframework.ai.tool.annotation.ToolParam;
@@ -26,6 +29,7 @@ public class CoreMcp implements McpService.McpImpl
2629
{
2730
@Tool(description = "This tool provides useful context information about the current user (name, userid), webserver " +
2831
"(name, url, description), and current container/folder (name, path, url, description) once the container is set via setContainer.")
32+
@RequiresPermission(ReadPermission.class)
2933
String whereAmIWhoAmITalkingTo(ToolContext context)
3034
{
3135
var cu = getContext(context);
@@ -70,6 +74,7 @@ String whereAmIWhoAmITalkingTo(ToolContext context)
7074
}
7175

7276
@Tool(description = "List the hierarchical path for every container in the server where the user has read permissions.")
77+
@RequiresNoPermission
7378
String listContainers(ToolContext toolContext)
7479
{
7580
return ContainerManager.getAllChildren(ContainerManager.getRoot(), getUser(toolContext), ReadPermission.class)
@@ -80,8 +85,10 @@ String listContainers(ToolContext toolContext)
8085
}
8186

8287
@Tool(description = "Every tool in this MCP requires a container path, e.g. MyProject/MyFolder. A container is also called a folder or project. " +
83-
"Please prompt the user for a container path and use this tool to save the path for this session. Don't suggest a leading slash on the path " +
84-
"because typing a slash in some LLM clients triggers custom shortcuts.")
88+
"Please prompt the user for a container path and use this tool to save the path for this MCP session. The user can also change the container " +
89+
"during the session using this tool. The user must have read permissions in the container, in other words, the path must be on the list that " +
90+
"the listContainers tool returns. Don't suggest a leading slash on the path because typing a slash in some LLM clients triggers custom shortcuts.")
91+
@RequiresNoPermission // Because we don't have a container yet, but tool will check for read permission before setting the container
8592
String setContainer(ToolContext context, @ToolParam(description = "Container path, e.g. MyProject/MyFolder") String containerPath)
8693
{
8794
final String message;
@@ -100,6 +107,10 @@ String setContainer(ToolContext context, @ToolParam(description = "Container pat
100107
}
101108
else
102109
{
110+
// Must have read permission to set a container
111+
if (!container.hasPermission(getUser(context), ReadPermission.class))
112+
throw new UnauthorizedException();
113+
103114
McpService.get().saveSessionContainer(context, container);
104115
message = "Container has been set to " + container.getPath();
105116
}

query/src/org/labkey/query/controllers/QueryMcp.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import org.labkey.api.query.SchemaKey;
2020
import org.labkey.api.query.SimpleSchemaTreeVisitor;
2121
import org.labkey.api.query.UserSchema;
22+
import org.labkey.api.security.RequiresPermission;
23+
import org.labkey.api.security.permissions.ReadPermission;
2224
import org.labkey.api.view.NotFoundException;
2325
import org.labkey.api.writer.ContainerUser;
2426
import org.labkey.query.sql.SqlParser;
@@ -44,7 +46,7 @@ public class QueryMcp implements McpService.McpImpl
4446
description = "Provide documentation for LabKey SQL specific syntax")
4547
public McpSchema.ReadResourceResult getLabKeySQLDocumentation() throws IOException
4648
{
47-
incrementResourceReadCount("LabKey SQL");
49+
incrementResourceRequestCount("LabKey SQL");
4850
String markdown = IOUtils.resourceToString("org/labkey/query/controllers/LabKeySql.md", null, QueryController.class.getClassLoader());
4951
return new McpSchema.ReadResourceResult(List.of(
5052
new McpSchema.TextResourceContents(
@@ -56,20 +58,23 @@ public McpSchema.ReadResourceResult getLabKeySQLDocumentation() throws IOExcepti
5658
}
5759

5860
@Tool(description = "Provide column metadata for a sql table. This tool will also return SQL source for saved queries.")
61+
@RequiresPermission(ReadPermission.class)
5962
String listColumns(ToolContext toolContext, @ToolParam(description = "Fully qualified table name as it would appear in SQL e.g. \"schema\".\"table\"") String fullQuotedTableName)
6063
{
6164
var json = _listColumns(fullQuotedTableName, toolContext);
6265
return json.toString();
6366
}
6467

6568
@Tool(description = "Provide list of tables within the provided schema.")
69+
@RequiresPermission(ReadPermission.class)
6670
String listTables(ToolContext toolContext, @ToolParam(description = "Fully qualified schema name as it would appear in SQL e.g. \"schema\"") String quotedSchemaName)
6771
{
6872
var json = _listTables(quotedSchemaName, getContext(toolContext));
6973
return json.toString();
7074
}
7175

7276
@Tool(description = "Provide list of database schemas")
77+
@RequiresPermission(ReadPermission.class)
7378
String listSchemas(ToolContext toolContext)
7479
{
7580
ContainerUser cu = getContext(toolContext);
@@ -88,6 +93,7 @@ String listSchemas(ToolContext toolContext)
8893

8994

9095
@Tool(description = "Provide the SQL source for a saved query.")
96+
@RequiresPermission(ReadPermission.class)
9197
String getSourceForSavedQuery(ToolContext toolContext, @ToolParam(description = "Fully qualified query name as it would appear in SQL e.g. \"schema\".\"table or query\"") String fullQuotedTableName)
9298
{
9399
var json = _listTables(fullQuotedTableName, getContext(toolContext));
@@ -309,7 +315,6 @@ static String normalizeIdentifier(String compoundIdentifier)
309315
return new SqlParser().parseIdentifier(compoundIdentifier).toSQLString(true).toLowerCase();
310316
}
311317

312-
313318
/** JSON schema example provided by GEMINI, using triple tick-marks to delimit the machine-readable structured data
314319
*
315320
* Here is the database schema in JSON format:
@@ -338,4 +343,3 @@ static String normalizeIdentifier(String compoundIdentifier)
338343
* }```
339344
*/
340345
}
341-

0 commit comments

Comments
 (0)