From a1221ac02d3f4ae115aaa3db16ea64c3aec8f465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=AA=E7=A8=8B=E4=BC=9F?= Date: Wed, 1 Jul 2026 07:15:00 +0800 Subject: [PATCH] feat(mcp): type on-behalf-of identity by channel/trust (#459) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The identity forwarded to opt-in MCP servers was a one-dimensional string (ChatOrigin.requesterId): a MateClaw username for web logins, but a webchat visitorId for visitors and an IM sender id for IM — indistinguishable to the REST backend. The signed-token mode (d204b702) made this worse: an RS256 signature over an unauthenticated visitorId reads as "MateClaw authenticated this user" to any backend that trusts the signature. Introduce an identity-typing dimension at McpIdentityForwardService: - classify() branches on ChatOrigin: authenticated (web login, sub=immutable userId), anonymous (webchat visitor, trust=anonymous), external (IM sender, trust=external), or none (cron/system → nothing injected, fail-closed). - mint() adds `trust` and `channel_type` claims; plaintext value is prefixed `trust:subject` so backends can tell the kinds apart without a JWT. The immutable userId reaches resolve() without coupling it to the user store: JwtAuthFilter stamps user.id into auth.setDetails() (both JWT and PAT paths), and ChatController.memoryOrigin carries it on a new ChatOrigin.requesterUserId field (only-add, per the record's evolution rule). Resolves the webchat semantic mismatch raised in #459 and the "sub should be an immutable user id" follow-up. 82 tests green (4 identity classes covered with claim assertions + full ChatOrigin/MCP regression). (cherry picked from commit b5d2cfbf98b39848d7139c743a0b81fea71e8ffe) --- .../vip/mate/agent/context/ChatOrigin.java | 43 +++- .../channel/ChannelChatOriginFactory.java | 3 +- .../vip/mate/channel/web/ChatController.java | 24 ++- .../java/vip/mate/config/JwtAuthFilter.java | 2 + .../lifecycle/SkillConsolidationService.java | 2 +- .../reflection/SkillReflectionService.java | 2 +- .../runtime/McpIdentityForwardService.java | 108 ++++++++-- .../context/ChatOriginSenderFieldsTest.java | 4 +- .../mate/agent/context/ChatOriginTest.java | 4 +- .../RuntimeContextInjectorModelTest.java | 2 +- .../RuntimeContextInjectorSenderTest.java | 6 +- .../ApprovalReplayContinuityTest.java | 2 +- .../cron/service/CronJobRunnerPromptTest.java | 2 +- ...elegateAsyncTaskOutputAttributionTest.java | 2 +- .../tool/builtin/DelegateAsyncToolTest.java | 2 +- .../McpIdentityForwardServiceTest.java | 198 ++++++++++++++---- 16 files changed, 324 insertions(+), 82 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/agent/context/ChatOrigin.java b/mateclaw-server/src/main/java/vip/mate/agent/context/ChatOrigin.java index 5e2e3a17..46118d6a 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/context/ChatOrigin.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/context/ChatOrigin.java @@ -66,7 +66,16 @@ public record ChatOrigin( * can still mint absolute download links. Null for IM/cron origins, which * have no request host; those rely on {@code mateclaw.server.public-base-url}. */ - @Nullable String baseUrl + @Nullable String baseUrl, + /** + * Immutable numeric id of the MateClaw user behind this request, when the + * requester is an authenticated account (JWT/PAT login via the + * web console). Null for non-account origins — webchat visitors, IM + * senders, cron — which carry no MateClaw user row. On-behalf-of identity + * forwarding uses this to tell "MateClaw authenticated this user" apart + * from "this is an external/anonymous identifier" (RFC: identity typing). + */ + @Nullable Long requesterUserId ) { /** Key used when this origin is wrapped into a Spring AI {@link ToolContext}. */ @@ -74,7 +83,7 @@ public record ChatOrigin( /** Sentinel used by AgentService default overloads where no origin is supplied. */ public static final ChatOrigin EMPTY = - new ChatOrigin(null, null, "", null, null, null, null, false, null, null, null, null); + new ChatOrigin(null, null, "", null, null, null, null, false, null, null, null, null, null); // ---------------- Factories per entry point ---------------- @@ -90,9 +99,25 @@ public record ChatOrigin( @Nullable Long workspaceId, @Nullable String workspaceBasePath, @Nullable String baseUrl) { + return web(conversationId, requesterId, workspaceId, workspaceBasePath, baseUrl, null); + } + + /** + * Web-console origin that also carries the authenticated user's immutable + * numeric id. Use this overload from the authenticated web entry point so + * on-behalf-of identity forwarding can assert "MateClaw authenticated this + * user" rather than an external/anonymous identifier. + */ + public static ChatOrigin web(@Nullable String conversationId, + @Nullable String requesterId, + @Nullable Long workspaceId, + @Nullable String workspaceBasePath, + @Nullable String baseUrl, + @Nullable Long requesterUserId) { return new ChatOrigin(null, conversationId, requesterId != null ? requesterId : "", - workspaceId, workspaceBasePath, null, null, false, null, "web", null, baseUrl); + workspaceId, workspaceBasePath, null, null, false, null, "web", null, baseUrl, + requesterUserId); } public static ChatOrigin cron(@Nullable String conversationId, @@ -101,7 +126,7 @@ public record ChatOrigin( @Nullable Long channelId, @Nullable ChannelTarget target) { return new ChatOrigin(null, conversationId, "system", - workspaceId, workspaceBasePath, channelId, target, true, null, null, null, null); + workspaceId, workspaceBasePath, channelId, target, true, null, null, null, null, null); } // ---------------- Wither-style updates ---------------- @@ -109,27 +134,27 @@ public record ChatOrigin( public ChatOrigin withAgent(@Nullable Long newAgentId) { return new ChatOrigin(newAgentId, conversationId, requesterId, workspaceId, workspaceBasePath, channelId, channelTarget, cronOrigin, - senderName, channelType, chatId, baseUrl); + senderName, channelType, chatId, baseUrl, requesterUserId); } public ChatOrigin withWorkspace(@Nullable Long newWorkspaceId, @Nullable String newWorkspaceBasePath) { return new ChatOrigin(agentId, conversationId, requesterId, newWorkspaceId, newWorkspaceBasePath, channelId, channelTarget, cronOrigin, - senderName, channelType, chatId, baseUrl); + senderName, channelType, chatId, baseUrl, requesterUserId); } public ChatOrigin withConversationId(@Nullable String newConversationId) { return new ChatOrigin(agentId, newConversationId, requesterId, workspaceId, workspaceBasePath, channelId, channelTarget, cronOrigin, - senderName, channelType, chatId, baseUrl); + senderName, channelType, chatId, baseUrl, requesterUserId); } /** Carry a request-derived public base URL (see {@link #baseUrl()}). */ public ChatOrigin withBaseUrl(@Nullable String newBaseUrl) { return new ChatOrigin(agentId, conversationId, requesterId, workspaceId, workspaceBasePath, channelId, channelTarget, cronOrigin, - senderName, channelType, chatId, newBaseUrl); + senderName, channelType, chatId, newBaseUrl, requesterUserId); } /** @@ -143,7 +168,7 @@ public record ChatOrigin( @Nullable String newChatId) { return new ChatOrigin(agentId, conversationId, requesterId, workspaceId, workspaceBasePath, channelId, channelTarget, cronOrigin, - newSenderName, newChannelType, newChatId, baseUrl); + newSenderName, newChannelType, newChatId, baseUrl, requesterUserId); } // ---------------- Spring AI ToolContext interop ---------------- diff --git a/mateclaw-server/src/main/java/vip/mate/channel/ChannelChatOriginFactory.java b/mateclaw-server/src/main/java/vip/mate/channel/ChannelChatOriginFactory.java index f8b166d4..7a4dbd90 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/ChannelChatOriginFactory.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/ChannelChatOriginFactory.java @@ -44,7 +44,8 @@ public class ChannelChatOriginFactory { ? message.getChannelType() : channel.getChannelType(), /* chatId */ message.getChatId(), - /* baseUrl */ null); // IM origins have no request host; rely on public-base-url config + /* baseUrl */ null, // IM origins have no request host; rely on public-base-url config + /* requesterUserId */ null); // IM senders are external platform ids, not MateClaw accounts } /** diff --git a/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java b/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java index 94a0419e..877d13b3 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java @@ -556,7 +556,7 @@ public class ChatController { // tools that need a workspace path read it from the agent (origin // is enriched with workspaceBasePath in StateGraph buildInitialState). vip.mate.agent.context.ChatOrigin webOrigin = - memoryOrigin(conversationId, username, workspaceId, request.getEndUserId()) + memoryOrigin(conversationId, username, requesterUserIdOf(auth), workspaceId, request.getEndUserId()) .withBaseUrl(requestBaseUrl); Disposable disposable = agentService.chatStructuredStream(agentId, promptText, conversationId, username, request.getThinkingLevel(), webOrigin) .doOnNext(delta -> { @@ -1057,7 +1057,7 @@ public class ChatController { // Carry the web origin so per-owner memory recall (read) and the // post-conversation memory write below agree on the same owner key. vip.mate.agent.context.ChatOrigin webOrigin = - memoryOrigin(request.getConversationId(), username, workspaceId, request.getEndUserId()); + memoryOrigin(request.getConversationId(), username, requesterUserIdOf(auth), workspaceId, request.getEndUserId()); AgentService.ChatResult result = agentService.chatWithUsage(agentId, promptText, request.getConversationId(), webOrigin); String response = result.content(); conversationService.saveMessage(request.getConversationId(), "assistant", response, null, "completed", @@ -1170,17 +1170,33 @@ public class ChatController { * MateClaw user ({@code user:}). */ private vip.mate.agent.context.ChatOrigin memoryOrigin(String conversationId, String username, - Long workspaceId, String endUserId) { + Long requesterUserId, Long workspaceId, + String endUserId) { // Resolve the public base URL here, on the request thread, so it can ride // the origin into async tool execution where no request is bound. Tools // then mint absolute download links without operator config. String baseUrl = resolveRequestBaseUrl(); if (endUserId != null && !endUserId.isBlank()) { + // Third-party single-account integration: the requester is an external + // end-user id, not a MateClaw account — no requesterUserId to assert. return vip.mate.agent.context.ChatOrigin .web(conversationId, endUserId.trim(), workspaceId, null, baseUrl) .withSender(null, "api", null); } - return vip.mate.agent.context.ChatOrigin.web(conversationId, username, workspaceId, null, baseUrl); + // Authenticated web user: carry the immutable id so on-behalf-of identity + // forwarding can assert "MateClaw authenticated this user" (not an anon id). + return vip.mate.agent.context.ChatOrigin.web(conversationId, username, workspaceId, null, baseUrl, requesterUserId); + } + + /** + * Extract the authenticated user's immutable numeric id from the + * {@link Authentication} details (stamped by {@code JwtAuthFilter} for both + * the JWT and PAT paths). Null when not authenticated or details absent. + */ + private Long requesterUserIdOf(org.springframework.security.core.Authentication auth) { + if (auth == null) return null; + Object details = auth.getDetails(); + return details instanceof Long id ? id : null; } /** diff --git a/mateclaw-server/src/main/java/vip/mate/config/JwtAuthFilter.java b/mateclaw-server/src/main/java/vip/mate/config/JwtAuthFilter.java index 8a5b17ab..71aaf2a1 100644 --- a/mateclaw-server/src/main/java/vip/mate/config/JwtAuthFilter.java +++ b/mateclaw-server/src/main/java/vip/mate/config/JwtAuthFilter.java @@ -79,6 +79,7 @@ public class JwtAuthFilter extends OncePerRequestFilter { user.getUsername(), null, List.of(new SimpleGrantedAuthority("ROLE_" + user.getRole().toUpperCase())) ); + auth.setDetails(user.getId()); // immutable user id for on-behalf-of forwarding SecurityContextHolder.getContext().setAuthentication(auth); patService.recordUse(pat); // debounced inside the service } catch (Exception ignored) { @@ -99,6 +100,7 @@ public class JwtAuthFilter extends OncePerRequestFilter { username, null, List.of(new SimpleGrantedAuthority("ROLE_" + user.getRole().toUpperCase())) ); + auth.setDetails(user.getId()); // immutable user id for on-behalf-of forwarding SecurityContextHolder.getContext().setAuthentication(auth); // 滑动窗口续期：Token 接近过期时自动签发新 Token diff --git a/mateclaw-server/src/main/java/vip/mate/skill/lifecycle/SkillConsolidationService.java b/mateclaw-server/src/main/java/vip/mate/skill/lifecycle/SkillConsolidationService.java index 0df55269..e35f4ba6 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/lifecycle/SkillConsolidationService.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/lifecycle/SkillConsolidationService.java @@ -205,7 +205,7 @@ public class SkillConsolidationService { private ToolContext toolContext(String sourceConversationId) { ChatOrigin origin = new ChatOrigin(null, sourceConversationId, "", null, null, - null, null, false, null, null, null, null); + null, null, false, null, null, null, null, null); return new ToolContext(Map.of(ChatOrigin.CTX_KEY, origin)); } diff --git a/mateclaw-server/src/main/java/vip/mate/skill/reflection/SkillReflectionService.java b/mateclaw-server/src/main/java/vip/mate/skill/reflection/SkillReflectionService.java index ae9efe50..a3710cf4 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/reflection/SkillReflectionService.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/reflection/SkillReflectionService.java @@ -214,7 +214,7 @@ public class SkillReflectionService { */ private ToolContext buildToolContext(Long agentId, String conversationId) { ChatOrigin origin = new ChatOrigin(agentId, conversationId, "", null, null, - null, null, false, null, null, null, null); + null, null, false, null, null, null, null, null); return new ToolContext(Map.of(ChatOrigin.CTX_KEY, origin)); } diff --git a/mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpIdentityForwardService.java b/mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpIdentityForwardService.java index 636407b6..07690792 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpIdentityForwardService.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpIdentityForwardService.java @@ -4,6 +4,7 @@ import io.jsonwebtoken.Jwts; import lombok.extern.slf4j.Slf4j; import org.springframework.ai.chat.model.ToolContext; import org.springframework.stereotype.Service; +import vip.mate.agent.context.ChatOrigin; import vip.mate.tool.builtin.ToolExecutionContext; import java.security.KeyFactory; @@ -20,19 +21,31 @@ import java.util.UUID; * Resolves what identity to inject into an opt-in MCP server's tool call, and * (in token mode) mints the signed assertion. * - *

Two modes, per {@link McpIdentityForwardProperties}: + *

Identity typing. Not every requester is a MateClaw-authenticated + * account. The resolved identity is typed so the REST backend can tell + * "MateClaw authenticated this user" apart from "this is an external/anonymous + * identifier" (see RFC: on-behalf-of identity typing): *

* - *

The {@code sub} carries the MateClaw user identifier - * ({@code ChatOrigin.requesterId}). If your backend keys authorization on an - * immutable numeric id, resolve username→id before minting (left as a refinement - * so this layer stays decoupled from the user store). + *

Two transport modes carry the typed identity, per {@link McpIdentityForwardProperties}: + *

* *

Fail-closed: when token mode is enabled but the signing key is missing or * unparseable, nothing is injected (the call goes out without identity and the @@ -44,6 +57,11 @@ import java.util.UUID; @Service public class McpIdentityForwardService { + /** Trust levels carried in the JWT {@code trust} claim / plaintext prefix. */ + static final String TRUST_AUTHENTICATED = "authenticated"; + static final String TRUST_ANONYMOUS = "anonymous"; + static final String TRUST_EXTERNAL = "external"; + private final McpIdentityForwardProperties properties; /** Lazily parsed signing key; {@code null} until first use / when unavailable. */ @@ -65,20 +83,70 @@ public class McpIdentityForwardService { /** The (key, value) to merge into the call arguments, or empty to inject nothing. */ public record Injection(String key, String value) {} + /** A typed identity resolved from the request origin. Empty = inject nothing. */ + record ResolvedIdentity(String subject, String trust, String channelType) { + static final ResolvedIdentity NONE = new ResolvedIdentity(null, null, null); + boolean present() { return subject != null && !subject.isBlank(); } + } + /** - * Resolve the identity injection for a call. Empty when there is no - * authenticated user (never fabricate identity), or when token mode is - * enabled but the key is unavailable (fail-closed). + * Classify the requester behind a tool call into a typed identity. Returns + * {@link ResolvedIdentity#NONE} for cron / system / unattributed origins + * (never assert identity on behalf of a non-user). + * + *

Source channel is read from {@link ChatOrigin} (carried on the + * {@link ToolContext}); when the context carries no origin it falls back to + * the legacy {@link ToolExecutionContext} username (treated as web/authenticated + * only if non-blank — preserves the original contract for the ThreadLocal path). + */ + ResolvedIdentity classify(ToolContext ctx) { + ChatOrigin origin = ChatOrigin.from(ctx); + // Cron / system / unattributed: never forward identity. + if (origin.cronOrigin() || "system".equals(origin.requesterId())) { + return ResolvedIdentity.NONE; + } + String channel = origin.channelType(); + // Authenticated web account: MateClaw vouches for this user. Prefer the + // immutable numeric id when available (web-console login); fall back to + // the username when only the ThreadLocal path supplied identity. + if (channel == null || channel.isBlank() || "web".equals(channel)) { + if (origin.requesterUserId() != null) { + return new ResolvedIdentity(String.valueOf(origin.requesterUserId()), + TRUST_AUTHENTICATED, "web"); + } + String user = ToolExecutionContext.username(ctx); + return user != null && !user.isBlank() + ? new ResolvedIdentity(user, TRUST_AUTHENTICATED, "web") + : ResolvedIdentity.NONE; + } + // webchat visitor ("api") or IM sender (feishu/wecom/…): external id, + // no MateClaw account — forward with an explicit trust downgrade so the + // backend knows it is NOT an authenticated MateClaw user. + String requester = origin.requesterId(); + if (requester == null || requester.isBlank()) { + return ResolvedIdentity.NONE; + } + String trust = "api".equals(channel) ? TRUST_ANONYMOUS : TRUST_EXTERNAL; + return new ResolvedIdentity(requester, trust, channel); + } + + /** + * Resolve the identity injection for a call. Empty when the origin carries + * no usable identity (cron / system / anonymous-without-id), or when token + * mode is enabled but the key is unavailable (fail-closed). */ public Optional resolve(ToolContext ctx, String audience) { - String user = ToolExecutionContext.username(ctx); - if (user == null || user.isBlank()) { + ResolvedIdentity id = classify(ctx); + if (!id.present()) { return Optional.empty(); } if (!properties.getToken().isEnabled()) { - return Optional.of(new Injection(McpIdentityForwardProperties.USER_ARG, user)); + // Plaintext: prefix the value with the trust level so the backend + // can tell authenticated from anonymous/external without a JWT. + return Optional.of(new Injection(McpIdentityForwardProperties.USER_ARG, + id.trust() + ":" + id.subject())); } - String jwt = mint(user, audience); + String jwt = mint(id, audience); if (jwt == null) { return Optional.empty(); // fail-closed: token mode but no key } @@ -86,7 +154,7 @@ public class McpIdentityForwardService { } /** Mint a short-lived RS256 JWT, or {@code null} if the key is unavailable. */ - private String mint(String subject, String audience) { + private String mint(ResolvedIdentity id, String audience) { PrivateKey key = signingKey(); if (key == null) { return null; @@ -97,8 +165,10 @@ public class McpIdentityForwardService { return Jwts.builder() .header().keyId(t.getKeyId()).and() .issuer(t.getIssuer()) - .subject(subject) + .subject(id.subject()) .audience().add(audience).and() + .claim("trust", id.trust()) + .claim("channel_type", id.channelType()) .id(UUID.randomUUID().toString()) .issuedAt(Date.from(now)) .expiration(Date.from(now.plus(Duration.ofSeconds(Math.max(1, t.getTtlSeconds()))))) diff --git a/mateclaw-server/src/test/java/vip/mate/agent/context/ChatOriginSenderFieldsTest.java b/mateclaw-server/src/test/java/vip/mate/agent/context/ChatOriginSenderFieldsTest.java index ba5b84eb..0ec4292d 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/context/ChatOriginSenderFieldsTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/context/ChatOriginSenderFieldsTest.java @@ -41,7 +41,7 @@ class ChatOriginSenderFieldsTest { void withSenderPreservesOtherFields() { ChatOrigin original = new ChatOrigin( 7L, "conv-1", "u123", 5L, "/ws", 9L, null, false, - null, null, null, null); + null, null, null, null, null); ChatOrigin enriched = original.withSender("Alice", "wecom", "g-1"); // All non-sender fields unchanged @@ -80,7 +80,7 @@ class ChatOriginSenderFieldsTest { ChatOrigin origin = new ChatOrigin( 7L, "feishu:oc_42", "ou_xyz", 5L, "/data/ws/5", 9L, null, false, - "Alice", "feishu", "oc_42", null); + "Alice", "feishu", "oc_42", null, null); String json = om.writeValueAsString(origin); ChatOrigin restored = om.readValue(json, ChatOrigin.class); diff --git a/mateclaw-server/src/test/java/vip/mate/agent/context/ChatOriginTest.java b/mateclaw-server/src/test/java/vip/mate/agent/context/ChatOriginTest.java index faf1c71f..129722d2 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/context/ChatOriginTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/context/ChatOriginTest.java @@ -28,7 +28,7 @@ class ChatOriginTest { void roundTripThroughToolContext_preservesAllFields() { ChannelTarget target = new ChannelTarget("user-42", "thread-abc", "bot-001"); ChatOrigin original = new ChatOrigin(7L, "wechat:42", "u123", 5L, - "/data/ws/5", 9L, target, false, null, null, null, null); + "/data/ws/5", 9L, target, false, null, null, null, null, null); ToolContext ctx = original.toToolContext(); ChatOrigin restored = ChatOrigin.from(ctx); @@ -75,7 +75,7 @@ class ChatOriginTest { void jsonSerialization_isStableAndForwardCompatible() throws Exception { ObjectMapper om = new ObjectMapper(); ChatOrigin origin = new ChatOrigin(7L, "wechat:42", "u123", 5L, - "/data/ws/5", 9L, new ChannelTarget("user-42", "thread-abc", "bot-001"), false, null, null, null, null); + "/data/ws/5", 9L, new ChannelTarget("user-42", "thread-abc", "bot-001"), false, null, null, null, null, null); String json = om.writeValueAsString(origin); ChatOrigin restored = om.readValue(json, ChatOrigin.class); diff --git a/mateclaw-server/src/test/java/vip/mate/agent/context/RuntimeContextInjectorModelTest.java b/mateclaw-server/src/test/java/vip/mate/agent/context/RuntimeContextInjectorModelTest.java index 0ce8012f..b6af4903 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/context/RuntimeContextInjectorModelTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/context/RuntimeContextInjectorModelTest.java @@ -65,7 +65,7 @@ class RuntimeContextInjectorModelTest { void imOriginHasSenderAndModel() { ChatOrigin origin = new ChatOrigin( 7L, "feishu:oc_abc", "ou_xyz", 5L, "/data/ws/5", - 9L, null, false, "Alice", "feishu", "oc_abc", null); + 9L, null, false, "Alice", "feishu", "oc_abc", null, null); String ctx = RuntimeContextInjector.buildContextMessage( "/data/ws/5", null, origin, "gpt-4o", "openai"); diff --git a/mateclaw-server/src/test/java/vip/mate/agent/context/RuntimeContextInjectorSenderTest.java b/mateclaw-server/src/test/java/vip/mate/agent/context/RuntimeContextInjectorSenderTest.java index c4d23f2f..65c79d56 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/context/RuntimeContextInjectorSenderTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/context/RuntimeContextInjectorSenderTest.java @@ -28,7 +28,7 @@ class RuntimeContextInjectorSenderTest { /* senderName */ "Alice", /* channelType */ "feishu", /* chatId */ "oc_abc", - /* baseUrl */ null); + /* baseUrl */ null, null); String ctx = RuntimeContextInjector.buildContextMessage("/data/ws/5", null, origin); @@ -45,7 +45,7 @@ class RuntimeContextInjectorSenderTest { ChatOrigin origin = new ChatOrigin( 7L, "feishu:ou_xyz", "ou_xyz", 5L, "/data/ws/5", 9L, null, false, - "Alice", "feishu", null, null); + "Alice", "feishu", null, null, null); String ctx = RuntimeContextInjector.buildContextMessage("/data/ws/5", null, origin); @@ -102,7 +102,7 @@ class RuntimeContextInjectorSenderTest { void blankSenderName() { ChatOrigin origin = new ChatOrigin( 7L, null, "ou_xyz", null, null, null, null, false, - /* senderName */ " ", "feishu", null, null); + /* senderName */ " ", "feishu", null, null, null); String ctx = RuntimeContextInjector.buildContextMessage(null, null, origin); diff --git a/mateclaw-server/src/test/java/vip/mate/approval/ApprovalReplayContinuityTest.java b/mateclaw-server/src/test/java/vip/mate/approval/ApprovalReplayContinuityTest.java index 4da63d74..4f925857 100644 --- a/mateclaw-server/src/test/java/vip/mate/approval/ApprovalReplayContinuityTest.java +++ b/mateclaw-server/src/test/java/vip/mate/approval/ApprovalReplayContinuityTest.java @@ -45,7 +45,7 @@ class ApprovalReplayContinuityTest { /* senderName */ "Alice", /* channelType */ "wecom", /* chatId */ "group-a", - /* baseUrl */ null); + /* baseUrl */ null, null); String json = objectMapper.writeValueAsString(original); ChatOrigin restored = workflow.restoreChatOrigin(json); diff --git a/mateclaw-server/src/test/java/vip/mate/cron/service/CronJobRunnerPromptTest.java b/mateclaw-server/src/test/java/vip/mate/cron/service/CronJobRunnerPromptTest.java index 38cbe5ea..cb3ace85 100644 --- a/mateclaw-server/src/test/java/vip/mate/cron/service/CronJobRunnerPromptTest.java +++ b/mateclaw-server/src/test/java/vip/mate/cron/service/CronJobRunnerPromptTest.java @@ -42,7 +42,7 @@ class CronJobRunnerPromptTest { /* senderName */ null, /* channelType */ "feishu", /* chatId */ "group-a", - /* baseUrl */ null); + /* baseUrl */ null, null); String prompt = CronJobRunner.buildCronPrompt("提醒喝水", channelOrigin); assertTrue(prompt.contains("[定时任务执行说明]")); diff --git a/mateclaw-server/src/test/java/vip/mate/tool/builtin/DelegateAsyncTaskOutputAttributionTest.java b/mateclaw-server/src/test/java/vip/mate/tool/builtin/DelegateAsyncTaskOutputAttributionTest.java index 35dd9d5e..9c7d8389 100644 --- a/mateclaw-server/src/test/java/vip/mate/tool/builtin/DelegateAsyncTaskOutputAttributionTest.java +++ b/mateclaw-server/src/test/java/vip/mate/tool/builtin/DelegateAsyncTaskOutputAttributionTest.java @@ -196,7 +196,7 @@ class DelegateAsyncTaskOutputAttributionTest { private ToolContext makeCtx(String requester, String conversationId) { ChatOrigin origin = new ChatOrigin( - 1L, conversationId, requester, null, null, null, null, false, null, null, null, null); + 1L, conversationId, requester, null, null, null, null, false, null, null, null, null, null); Map map = new HashMap<>(); map.put(ChatOrigin.CTX_KEY, origin); return new ToolContext(map); diff --git a/mateclaw-server/src/test/java/vip/mate/tool/builtin/DelegateAsyncToolTest.java b/mateclaw-server/src/test/java/vip/mate/tool/builtin/DelegateAsyncToolTest.java index c1f5f0ac..acbe68b6 100644 --- a/mateclaw-server/src/test/java/vip/mate/tool/builtin/DelegateAsyncToolTest.java +++ b/mateclaw-server/src/test/java/vip/mate/tool/builtin/DelegateAsyncToolTest.java @@ -389,7 +389,7 @@ class DelegateAsyncToolTest { private ToolContext makeCtx(String requester, String conversationId) { ChatOrigin origin = new ChatOrigin( - 1L, conversationId, requester, null, null, null, null, false, null, null, null, null); + 1L, conversationId, requester, null, null, null, null, false, null, null, null, null, null); Map map = new HashMap<>(); map.put(ChatOrigin.CTX_KEY, origin); return new ToolContext(map); diff --git a/mateclaw-server/src/test/java/vip/mate/tool/mcp/runtime/McpIdentityForwardServiceTest.java b/mateclaw-server/src/test/java/vip/mate/tool/mcp/runtime/McpIdentityForwardServiceTest.java index 234f6176..e012a7df 100644 --- a/mateclaw-server/src/test/java/vip/mate/tool/mcp/runtime/McpIdentityForwardServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/tool/mcp/runtime/McpIdentityForwardServiceTest.java @@ -4,7 +4,10 @@ import io.jsonwebtoken.Claims; import io.jsonwebtoken.Jwts; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.springframework.ai.chat.model.ToolContext; +import vip.mate.agent.context.ChatOrigin; import vip.mate.tool.builtin.ToolExecutionContext; import java.security.KeyPair; @@ -17,9 +20,13 @@ import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; /** - * Tests {@link McpIdentityForwardService}: plaintext vs signed-token resolution, - * fail-closed when token mode lacks a key, and that a minted token verifies - * against the public key with the right claims. + * Tests {@link McpIdentityForwardService}: identity typing across channels, + * plaintext vs signed-token resolution, and fail-closed behaviour. + * + *

The core of this suite is the {@code trust}/{@code channel_type} typing — + * making sure an authenticated MateClaw user, a webchat visitor, an IM sender, + * and a cron run are forwarded with distinguishable, non-fabricated identities + * (see RFC: on-behalf-of identity typing, issue #459 review). */ class McpIdentityForwardServiceTest { @@ -32,50 +39,149 @@ class McpIdentityForwardServiceTest { return new McpIdentityForwardService(p); } - @Test - @DisplayName("plaintext mode: injects username under USER_ARG") - void plaintext() { - ToolExecutionContext.set("c1", "alice"); - var p = new McpIdentityForwardProperties(); - Optional inj = svc(p).resolve(null, "my-api"); - assertThat(inj).isPresent(); - assertThat(inj.get().key()).isEqualTo(McpIdentityForwardProperties.USER_ARG); - assertThat(inj.get().value()).isEqualTo("alice"); + /** Build a ToolContext carrying the given origin, mirroring ToolExecutionExecutor. */ + private static ToolContext ctx(ChatOrigin origin) { + return new ToolContext(Map.of(ChatOrigin.CTX_KEY, origin)); } - @Test - @DisplayName("no authenticated user: nothing injected") - void noUser() { - var p = new McpIdentityForwardProperties(); - assertThat(svc(p).resolve(null, "my-api")).isEmpty(); + private static KeyPair rsaKeyPair() { + try { + return KeyPairGenerator.getInstance("RSA").genKeyPair(); + } catch (Exception e) { + throw new IllegalStateException(e); + } } - @Test - @DisplayName("token mode but no key: fail-closed (nothing injected)") - void tokenModeNoKey() { - ToolExecutionContext.set("c1", "alice"); - var p = new McpIdentityForwardProperties(); - p.getToken().setEnabled(true); // no private-key-pem - assertThat(svc(p).resolve(null, "my-api")).isEmpty(); - } - - @Test - @DisplayName("token mode: mints an RS256 JWT that verifies with the public key") - void tokenMintAndVerify() throws Exception { - KeyPair kp = KeyPairGenerator.getInstance("RSA").genKeyPair(); // 2048 default + /** Token-mode config signed with {@code kp}'s private key. */ + private static McpIdentityForwardProperties tokenProps(KeyPair kp) { var p = new McpIdentityForwardProperties(); p.getToken().setEnabled(true); p.getToken().setIssuer("mateclaw"); p.getToken().setTtlSeconds(60); p.getToken().setPrivateKeyPem(Base64.getEncoder().encodeToString(kp.getPrivate().getEncoded())); + return p; + } - ToolExecutionContext.set("c1", "alice"); - Optional inj = svc(p).resolve(null, "my-api"); + // ==================== Identity typing across channels ==================== + @Nested + @DisplayName("classify: identity typing per channel") + class Classify { + + @Test + @DisplayName("authenticated web user → sub=userId, trust=authenticated") + void authenticatedWebUser() { + ChatOrigin origin = ChatOrigin.web("c1", "alice", 1L, null, null, 42L); + McpIdentityForwardService.ResolvedIdentity id = svc(new McpIdentityForwardProperties()).classify(ctx(origin)); + assertThat(id.subject()).isEqualTo("42"); + assertThat(id.trust()).isEqualTo(McpIdentityForwardService.TRUST_AUTHENTICATED); + assertThat(id.channelType()).isEqualTo("web"); + } + + @Test + @DisplayName("webchat visitor (channelType=api) → trust=anonymous, sub=visitorId") + void webchatVisitor() { + // webchat sets requesterId=visitorId and channelType=api via withSender + ChatOrigin origin = ChatOrigin.web("c1", "visitor-xyz", 1L, null) + .withSender(null, "api", null); + McpIdentityForwardService.ResolvedIdentity id = svc(new McpIdentityForwardProperties()).classify(ctx(origin)); + assertThat(id.subject()).isEqualTo("visitor-xyz"); + assertThat(id.trust()).isEqualTo(McpIdentityForwardService.TRUST_ANONYMOUS); + assertThat(id.channelType()).isEqualTo("api"); + } + + @Test + @DisplayName("IM sender (channelType=feishu) → trust=external") + void imSender() { + ChatOrigin origin = new ChatOrigin(null, "c1", "im_user_1", 1L, null, + 9L, null, false, "张三", "feishu", "grp1", null, null); + McpIdentityForwardService.ResolvedIdentity id = svc(new McpIdentityForwardProperties()).classify(ctx(origin)); + assertThat(id.subject()).isEqualTo("im_user_1"); + assertThat(id.trust()).isEqualTo(McpIdentityForwardService.TRUST_EXTERNAL); + assertThat(id.channelType()).isEqualTo("feishu"); + } + + @Test + @DisplayName("cron origin → NONE (never assert identity for a non-user)") + void cronOrigin() { + ChatOrigin origin = ChatOrigin.cron("c1", 1L, null, 9L, null); + assertThat(svc(new McpIdentityForwardProperties()).classify(ctx(origin))) + .isEqualTo(McpIdentityForwardService.ResolvedIdentity.NONE); + } + + @Test + @DisplayName("system requesterId → NONE") + void systemRequester() { + // An origin whose requesterId is "system" but cronOrigin=false (defensive) + ChatOrigin origin = new ChatOrigin(null, "c1", "system", 1L, null, + null, null, false, null, null, null, null, null); + assertThat(svc(new McpIdentityForwardProperties()).classify(ctx(origin))) + .isEqualTo(McpIdentityForwardService.ResolvedIdentity.NONE); + } + + @Test + @DisplayName("web origin without userId falls back to ThreadLocal username (legacy path)") + void webOriginLegacyThreadLocal() { + ToolExecutionContext.set("c1", "alice"); + // 5-arg web(): no requesterUserId → falls back to ThreadLocal + ChatOrigin origin = ChatOrigin.web("c1", "alice", 1L, null, null); + McpIdentityForwardService.ResolvedIdentity id = svc(new McpIdentityForwardProperties()).classify(ctx(origin)); + assertThat(id.subject()).isEqualTo("alice"); + assertThat(id.trust()).isEqualTo(McpIdentityForwardService.TRUST_AUTHENTICATED); + } + } + + // ==================== Resolution: plaintext & token modes ==================== + + @Test + @DisplayName("plaintext mode: injects ':' under USER_ARG") + void plaintextTyped() { + ChatOrigin origin = ChatOrigin.web("c1", "alice", 1L, null, null, 42L); + var p = new McpIdentityForwardProperties(); + Optional inj = svc(p).resolve(ctx(origin), "my-api"); + assertThat(inj).isPresent(); + assertThat(inj.get().key()).isEqualTo(McpIdentityForwardProperties.USER_ARG); + assertThat(inj.get().value()).isEqualTo("authenticated:42"); + } + + @Test + @DisplayName("plaintext mode: anonymous visitor carries trust=anonymous prefix") + void plaintextAnonymous() { + ChatOrigin origin = ChatOrigin.web("c1", "visitor-xyz", 1L, null).withSender(null, "api", null); + Optional inj = + svc(new McpIdentityForwardProperties()).resolve(ctx(origin), "my-api"); + assertThat(inj).isPresent(); + assertThat(inj.get().value()).startsWith("anonymous:visitor-xyz"); + } + + @Test + @DisplayName("no usable identity (cron): nothing injected") + void noIdentity() { + ChatOrigin origin = ChatOrigin.cron("c1", 1L, null, 9L, null); + assertThat(svc(new McpIdentityForwardProperties()).resolve(ctx(origin), "my-api")).isEmpty(); + } + + @Test + @DisplayName("token mode but no key: fail-closed (nothing injected)") + void tokenModeNoKey() { + ChatOrigin origin = ChatOrigin.web("c1", "alice", 1L, null, null, 42L); + var p = new McpIdentityForwardProperties(); + p.getToken().setEnabled(true); // no private-key-pem + assertThat(svc(p).resolve(ctx(origin), "my-api")).isEmpty(); + } + + @Test + @DisplayName("token mode: mints RS256 JWT that verifies with trust/channel_type claims") + void tokenMintAndVerify() throws Exception { + KeyPair kp = rsaKeyPair(); + var p = tokenProps(kp); + ChatOrigin origin = ChatOrigin.web("c1", "alice", 1L, null, null, 42L); + + Optional inj = svc(p).resolve(ctx(origin), "my-api"); assertThat(inj).isPresent(); assertThat(inj.get().key()).isEqualTo(McpIdentityForwardProperties.TOKEN_ARG); - // The REST backend would do exactly this: verify with the public key. + // The REST backend verifies with the public key and reads the typed claims. Claims claims = Jwts.parser() .verifyWith(kp.getPublic()) .requireIssuer("mateclaw") @@ -84,9 +190,31 @@ class McpIdentityForwardServiceTest { .parseSignedClaims(inj.get().value()) .getPayload(); - assertThat(claims.getSubject()).isEqualTo("alice"); + assertThat(claims.getSubject()).isEqualTo("42"); + assertThat(claims.get("trust", String.class)).isEqualTo(McpIdentityForwardService.TRUST_AUTHENTICATED); + assertThat(claims.get("channel_type", String.class)).isEqualTo("web"); assertThat(claims.getExpiration()).isAfter(new Date()); - assertThat(claims.getId()).isNotBlank(); // jti present + assertThat(claims.getId()).isNotBlank(); + } + + @Test + @DisplayName("token mode: anonymous visitor token carries trust=anonymous") + void tokenAnonymousVisitor() throws Exception { + KeyPair kp = rsaKeyPair(); + var p = tokenProps(kp); + ChatOrigin origin = ChatOrigin.web("c1", "visitor-xyz", 1L, null).withSender(null, "api", null); + + Optional inj = svc(p).resolve(ctx(origin), "my-api"); + assertThat(inj).isPresent(); + + Claims claims = Jwts.parser() + .verifyWith(kp.getPublic()) + .build() + .parseSignedClaims(inj.get().value()) + .getPayload(); + assertThat(claims.getSubject()).isEqualTo("visitor-xyz"); + assertThat(claims.get("trust", String.class)).isEqualTo(McpIdentityForwardService.TRUST_ANONYMOUS); + assertThat(claims.get("channel_type", String.class)).isEqualTo("api"); } @Test