mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-16 04:18:17 +08:00
fix(conversation): surface webchat visitor sessions in the admin console
WebChat conversations are owned by an external visitor principal (webchat:<visitorId>) 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
This commit is contained in:
parent
981c3d56d9
commit
7f4c62c3d6
@ -62,6 +62,17 @@ public class ConversationService {
|
|||||||
|
|
||||||
public static final String SYSTEM_USER = "system";
|
public static final String SYSTEM_USER = "system";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Owner prefix for webchat conversations, written as {@code webchat:<visitorId>}
|
||||||
|
* (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 ConversationMapper conversationMapper;
|
||||||
private final MessageMapper messageMapper;
|
private final MessageMapper messageMapper;
|
||||||
private final AgentMapper agentMapper;
|
private final AgentMapper agentMapper;
|
||||||
@ -97,9 +108,29 @@ public class ConversationService {
|
|||||||
/**
|
/**
|
||||||
* Workspace-scoped variant of {@link #listConversations(String)}.
|
* Workspace-scoped variant of {@link #listConversations(String)}.
|
||||||
*
|
*
|
||||||
|
* <p>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.
|
||||||
|
*
|
||||||
* <p>获取用户的会话列表(按工作区过滤)。
|
* <p>获取用户的会话列表(按工作区过滤)。
|
||||||
*/
|
*/
|
||||||
public List<ConversationVO> listConversations(String username, Long workspaceId) {
|
public List<ConversationVO> 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:<visitorId>}) 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.
|
||||||
|
*
|
||||||
|
* <p>控制台变体:includeChannelPrincipals 为 true 时额外纳入 webchat 访客会话。
|
||||||
|
*/
|
||||||
|
public List<ConversationVO> listConversations(String username, Long workspaceId,
|
||||||
|
boolean includeChannelPrincipals) {
|
||||||
// Return both the current user's conversations AND those created by
|
// Return both the current user's conversations AND those created by
|
||||||
// scheduled jobs (owner=system). Child conversations spawned by
|
// scheduled jobs (owner=system). Child conversations spawned by
|
||||||
// delegation are excluded — they don't belong in the sidebar.
|
// delegation are excluded — they don't belong in the sidebar.
|
||||||
@ -107,7 +138,7 @@ public class ConversationService {
|
|||||||
// 同时返回当前用户的会话和定时任务(system)产生的会话;
|
// 同时返回当前用户的会话和定时任务(system)产生的会话;
|
||||||
// 排除子会话(委派产生的子会话不在侧边栏显示)。
|
// 排除子会话(委派产生的子会话不在侧边栏显示)。
|
||||||
LambdaQueryWrapper<ConversationEntity> wrapper = new LambdaQueryWrapper<ConversationEntity>()
|
LambdaQueryWrapper<ConversationEntity> wrapper = new LambdaQueryWrapper<ConversationEntity>()
|
||||||
.in(ConversationEntity::getUsername, username, SYSTEM_USER)
|
.and(w -> applyOwnerScope(w, username, includeChannelPrincipals))
|
||||||
.isNull(ConversationEntity::getParentConversationId)
|
.isNull(ConversationEntity::getParentConversationId)
|
||||||
.orderByDesc(ConversationEntity::getPinned)
|
.orderByDesc(ConversationEntity::getPinned)
|
||||||
.orderByDesc(ConversationEntity::getLastActiveTime);
|
.orderByDesc(ConversationEntity::getLastActiveTime);
|
||||||
@ -147,6 +178,20 @@ public class ConversationService {
|
|||||||
.collect(Collectors.toList());
|
.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<ConversationEntity> 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.
|
* Paginated variant used by the Sessions admin page.
|
||||||
*
|
*
|
||||||
@ -166,8 +211,10 @@ public class ConversationService {
|
|||||||
com.baomidou.mybatisplus.extension.plugins.pagination.Page<ConversationEntity> pager =
|
com.baomidou.mybatisplus.extension.plugins.pagination.Page<ConversationEntity> pager =
|
||||||
new com.baomidou.mybatisplus.extension.plugins.pagination.Page<>(page, size);
|
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<ConversationEntity> wrapper = new LambdaQueryWrapper<ConversationEntity>()
|
LambdaQueryWrapper<ConversationEntity> wrapper = new LambdaQueryWrapper<ConversationEntity>()
|
||||||
.in(ConversationEntity::getUsername, username, SYSTEM_USER)
|
.and(w -> applyOwnerScope(w, username, true))
|
||||||
.isNull(ConversationEntity::getParentConversationId)
|
.isNull(ConversationEntity::getParentConversationId)
|
||||||
.orderByDesc(ConversationEntity::getPinned)
|
.orderByDesc(ConversationEntity::getPinned)
|
||||||
.orderByDesc(ConversationEntity::getLastActiveTime);
|
.orderByDesc(ConversationEntity::getLastActiveTime);
|
||||||
@ -1325,11 +1372,13 @@ public class ConversationService {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Check whether a user owns the conversation, treating system-owned
|
* Check whether a user owns the conversation, treating system-owned
|
||||||
* rows (e.g. from scheduled jobs / IM channels) as visible to every
|
* rows (e.g. from scheduled jobs / IM channels) and external channel
|
||||||
* authenticated user.
|
* principals ({@code webchat:<visitorId>}) as visible to every
|
||||||
|
* authenticated user — otherwise the console could list a webchat
|
||||||
|
* conversation but 403 on opening / deleting / renaming it.
|
||||||
*
|
*
|
||||||
* <p>校验用户是否拥有该会话。定时任务产生的会话(username = system)
|
* <p>校验用户是否拥有该会话。定时任务产生的会话(username = system)
|
||||||
* 对所有登录用户可见。
|
* 和 webchat 访客会话(username = webchat:<visitorId>)对所有登录用户可见。
|
||||||
*/
|
*/
|
||||||
public boolean isConversationOwner(String conversationId, String username) {
|
public boolean isConversationOwner(String conversationId, String username) {
|
||||||
ConversationEntity conv = conversationMapper.selectOne(
|
ConversationEntity conv = conversationMapper.selectOne(
|
||||||
@ -1338,7 +1387,10 @@ public class ConversationService {
|
|||||||
if (conv == null) {
|
if (conv == null) {
|
||||||
return false;
|
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));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@ -38,7 +38,9 @@ public class ConversationController {
|
|||||||
Authentication auth,
|
Authentication auth,
|
||||||
@RequestHeader(value = "X-Workspace-Id", required = false) Long workspaceId) {
|
@RequestHeader(value = "X-Workspace-Id", required = false) Long workspaceId) {
|
||||||
String username = auth != null ? auth.getName() : "anonymous";
|
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));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@ -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.
|
||||||
|
*
|
||||||
|
* <p>WebChat threads are owned by an external visitor principal
|
||||||
|
* ({@code webchat:<visitorId>}), 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<LambdaQueryWrapper<ConversationEntity>> 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<LambdaQueryWrapper<ConversationEntity>> 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<LambdaQueryWrapper<ConversationEntity>> 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;
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue
Block a user