From 7f4c62c3d68158842c1fd8b3b8f2e121f6ec4531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=AA=E7=A8=8B=E4=BC=9F?= Date: Wed, 17 Jun 2026 21:03:02 +0800 Subject: [PATCH] fix(conversation): surface webchat visitor sessions in the admin console MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WebChat conversations are owned by an external visitor principal (webchat:) so each visitor's threads stay isolated for the self-service session API. But the console list/page/owner-check only recognized the current user + system owners, so these conversations were invisible in the sidebar and the Sessions page — and would 403 on open even if surfaced. Treat webchat: owners like system owners for the console: include them in the lenient list/page queries and in isConversationOwner. The strict listConversations overload (used by the visitor self-service path) is unchanged, so a visitor's own access is not widened. Refs matevip/mateclaw#340 --- .../conversation/ConversationService.java | 64 ++++++++- .../controller/ConversationController.java | 4 +- ...versationServiceWebchatVisibilityTest.java | 126 ++++++++++++++++++ 3 files changed, 187 insertions(+), 7 deletions(-) create mode 100644 mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceWebchatVisibilityTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java index 984e31dc..0b5f08b3 100644 --- a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java +++ b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java @@ -62,6 +62,17 @@ public class ConversationService { public static final String SYSTEM_USER = "system"; + /** + * Owner prefix for webchat conversations, written as {@code webchat:} + * (see {@code WebChatController#webchatUsername}). These rows are owned by an + * external visitor principal rather than a MateClaw account, so the admin + * console treats them like {@link #SYSTEM_USER} rows — visible to / manageable + * by any authenticated user in the workspace. The visitor-facing self-service + * endpoints keep isolating by the exact owner plus a signed visitor token, so + * surfacing these rows to the console does not widen a visitor's own access. + */ + static final String WEBCHAT_OWNER_PREFIX = "webchat:"; + private final ConversationMapper conversationMapper; private final MessageMapper messageMapper; private final AgentMapper agentMapper; @@ -97,9 +108,29 @@ public class ConversationService { /** * Workspace-scoped variant of {@link #listConversations(String)}. * + *

Strict ownership: only the user's own + {@code system} rows. Used by + * callers that must not see other principals' conversations — notably the + * webchat visitor self-service path, which scopes to one visitor. + * *

获取用户的会话列表（按工作区过滤）。 */ public List listConversations(String username, Long workspaceId) { + return listConversations(username, workspaceId, false); + } + + /** + * Admin-console variant. When {@code includeChannelPrincipals} is true, also + * returns conversations owned by external channel principals + * ({@code webchat:}) so the console surfaces webchat threads + * alongside the user's own + {@code system} rows — the same way IM-channel + * ({@code system}-owned) conversations already appear. The visitor-facing + * webchat endpoints keep using the strict overload, so this does not widen a + * visitor's own access. + * + *

控制台变体：includeChannelPrincipals 为 true 时额外纳入 webchat 访客会话。 + */ + public List listConversations(String username, Long workspaceId, + boolean includeChannelPrincipals) { // Return both the current user's conversations AND those created by // scheduled jobs (owner=system). Child conversations spawned by // delegation are excluded — they don't belong in the sidebar. @@ -107,7 +138,7 @@ public class ConversationService { // 同时返回当前用户的会话和定时任务（system）产生的会话； // 排除子会话（委派产生的子会话不在侧边栏显示）。 LambdaQueryWrapper wrapper = new LambdaQueryWrapper() - .in(ConversationEntity::getUsername, username, SYSTEM_USER) + .and(w -> applyOwnerScope(w, username, includeChannelPrincipals)) .isNull(ConversationEntity::getParentConversationId) .orderByDesc(ConversationEntity::getPinned) .orderByDesc(ConversationEntity::getLastActiveTime); @@ -147,6 +178,20 @@ public class ConversationService { .collect(Collectors.toList()); } + /** + * Apply the owner-scope predicate onto a (nested) wrapper: always the user's + * own + {@link #SYSTEM_USER} rows; when {@code includeChannelPrincipals} is + * true, also external channel-principal rows ({@code webchat:%}). Kept as one + * helper so the list and page queries stay in lockstep. + */ + private void applyOwnerScope(LambdaQueryWrapper w, + String username, boolean includeChannelPrincipals) { + w.in(ConversationEntity::getUsername, username, SYSTEM_USER); + if (includeChannelPrincipals) { + w.or().likeRight(ConversationEntity::getUsername, WEBCHAT_OWNER_PREFIX); + } + } + /** * Paginated variant used by the Sessions admin page. * @@ -166,8 +211,10 @@ public class ConversationService { com.baomidou.mybatisplus.extension.plugins.pagination.Page pager = new com.baomidou.mybatisplus.extension.plugins.pagination.Page<>(page, size); + // Admin Sessions page surfaces channel conversations too, so include + // external webchat principals alongside the user's own + system rows. LambdaQueryWrapper wrapper = new LambdaQueryWrapper() - .in(ConversationEntity::getUsername, username, SYSTEM_USER) + .and(w -> applyOwnerScope(w, username, true)) .isNull(ConversationEntity::getParentConversationId) .orderByDesc(ConversationEntity::getPinned) .orderByDesc(ConversationEntity::getLastActiveTime); @@ -1325,11 +1372,13 @@ public class ConversationService { /** * Check whether a user owns the conversation, treating system-owned - * rows (e.g. from scheduled jobs / IM channels) as visible to every - * authenticated user. + * rows (e.g. from scheduled jobs / IM channels) and external channel + * principals ({@code webchat:}) as visible to every + * authenticated user — otherwise the console could list a webchat + * conversation but 403 on opening / deleting / renaming it. * *

校验用户是否拥有该会话。定时任务产生的会话（username = system） - * 对所有登录用户可见。 + * 和 webchat 访客会话（username = webchat:<visitorId>）对所有登录用户可见。 */ public boolean isConversationOwner(String conversationId, String username) { ConversationEntity conv = conversationMapper.selectOne( @@ -1338,7 +1387,10 @@ public class ConversationService { if (conv == null) { return false; } - return username.equals(conv.getUsername()) || SYSTEM_USER.equals(conv.getUsername()); + String owner = conv.getUsername(); + return username.equals(owner) + || SYSTEM_USER.equals(owner) + || (owner != null && owner.startsWith(WEBCHAT_OWNER_PREFIX)); } /** diff --git a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/controller/ConversationController.java b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/controller/ConversationController.java index ab9a283c..2aa7c498 100644 --- a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/controller/ConversationController.java +++ b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/controller/ConversationController.java @@ -38,7 +38,9 @@ public class ConversationController { Authentication auth, @RequestHeader(value = "X-Workspace-Id", required = false) Long workspaceId) { String username = auth != null ? auth.getName() : "anonymous"; - return R.ok(conversationService.listConversations(username, workspaceId)); + // Admin console: include external channel principals (webchat visitors) + // so webchat threads show up alongside the user's own + system rows. + return R.ok(conversationService.listConversations(username, workspaceId, true)); } /** diff --git a/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceWebchatVisibilityTest.java b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceWebchatVisibilityTest.java new file mode 100644 index 00000000..13d6b660 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceWebchatVisibilityTest.java @@ -0,0 +1,126 @@ +package vip.mate.workspace.conversation; + +import com.baomidou.mybatisplus.core.MybatisConfiguration; +import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper; +import com.baomidou.mybatisplus.core.metadata.TableInfoHelper; +import com.baomidou.mybatisplus.extension.plugins.pagination.Page; +import org.apache.ibatis.builder.MapperBuilderAssistant; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import vip.mate.agent.repository.AgentMapper; +import vip.mate.workspace.conversation.model.ConversationEntity; +import vip.mate.workspace.conversation.repository.ConversationMapper; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +/** + * Pin the admin-console visibility of webchat conversations. + * + *

WebChat threads are owned by an external visitor principal + * ({@code webchat:}), not a MateClaw account. They must surface in + * the console list / page / owner-check the same way {@code system}-owned IM + * conversations do — otherwise they are silently invisible (the reported bug). + * The strict overload (used by the visitor self-service path) must NOT widen to + * other principals. + */ +@ExtendWith(MockitoExtension.class) +class ConversationServiceWebchatVisibilityTest { + + @Mock private ConversationMapper conversationMapper; + @Mock private AgentMapper agentMapper; + + @InjectMocks private ConversationService service; + + /** + * LambdaQueryWrapper resolves column names from MyBatis-Plus's table-info + * cache, which a Spring context would normally populate. Seed it directly so + * {@code getTargetSql()} / {@code getParamNameValuePairs()} work in this pure + * unit test. + */ + @BeforeAll + static void initLambdaCache() { + TableInfoHelper.initTableInfo( + new MapperBuilderAssistant(new MybatisConfiguration(), ""), + ConversationEntity.class); + } + + @Test + @DisplayName("lenient list includes webchat principals (username LIKE 'webchat:%')") + void lenientListIncludesWebchat() { + ArgumentCaptor> captor = + ArgumentCaptor.forClass(LambdaQueryWrapper.class); + when(conversationMapper.selectList(captor.capture())).thenReturn(List.of()); + + service.listConversations("admin", 1L, true); + + String sql = captor.getValue().getTargetSql(); + assertThat(sql).containsIgnoringCase("like"); + assertThat(captor.getValue().getParamNameValuePairs().values()) + .contains("webchat:%"); + } + + @Test + @DisplayName("strict list excludes webchat principals (no LIKE clause)") + void strictListExcludesWebchat() { + ArgumentCaptor> captor = + ArgumentCaptor.forClass(LambdaQueryWrapper.class); + when(conversationMapper.selectList(captor.capture())).thenReturn(List.of()); + + service.listConversations("admin", 1L); // strict 2-arg + + String sql = captor.getValue().getTargetSql(); + assertThat(sql).doesNotContainIgnoringCase("like"); + } + + @Test + @DisplayName("page query includes webchat principals") + void pageIncludesWebchat() { + ArgumentCaptor> captor = + ArgumentCaptor.forClass(LambdaQueryWrapper.class); + when(conversationMapper.selectPage(any(Page.class), captor.capture())) + .thenReturn(new Page<>()); + + service.pageConversations("admin", 1L, 1, 20, null); + + String sql = captor.getValue().getTargetSql(); + assertThat(sql).containsIgnoringCase("like"); + assertThat(captor.getValue().getParamNameValuePairs().values()) + .contains("webchat:%"); + } + + @Test + @DisplayName("isConversationOwner: webchat + system + self visible; foreign user not") + void ownerCheckRecognizesWebchat() { + when(conversationMapper.selectOne(any(LambdaQueryWrapper.class))) + .thenReturn(ownedBy("webchat:visitor-1")); + assertThat(service.isConversationOwner("webchat:k:visitor-1", "admin")).isTrue(); + + when(conversationMapper.selectOne(any(LambdaQueryWrapper.class))) + .thenReturn(ownedBy("system")); + assertThat(service.isConversationOwner("feishu:ou_x", "admin")).isTrue(); + + when(conversationMapper.selectOne(any(LambdaQueryWrapper.class))) + .thenReturn(ownedBy("admin")); + assertThat(service.isConversationOwner("c1", "admin")).isTrue(); + + when(conversationMapper.selectOne(any(LambdaQueryWrapper.class))) + .thenReturn(ownedBy("bob")); + assertThat(service.isConversationOwner("c2", "admin")).isFalse(); + } + + private static ConversationEntity ownedBy(String username) { + ConversationEntity conv = new ConversationEntity(); + conv.setUsername(username); + return conv; + } +}