mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 19:23:42 +08:00
feat(channel): scope conversation ids by channel to stop cross-workspace bleed
This commit is contained in:
parent
130112c5bf
commit
e294b32542
@ -268,7 +268,7 @@ public class ChannelMessageRouter {
|
|||||||
}
|
}
|
||||||
channelEntity = fresh;
|
channelEntity = fresh;
|
||||||
|
|
||||||
String conversationId = buildConversationId(message);
|
String conversationId = buildConversationId(message, channelEntity.getId());
|
||||||
if (handleMagicCommand(message, adapter, channelEntity, conversationId)) {
|
if (handleMagicCommand(message, adapter, channelEntity, conversationId)) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@ -468,7 +468,7 @@ public class ChannelMessageRouter {
|
|||||||
continue; // 超时,重新检查 shutdown 标志
|
continue; // 超时,重新检查 shutdown 标志
|
||||||
}
|
}
|
||||||
|
|
||||||
String conversationId = buildConversationId(entry.message());
|
String conversationId = buildConversationId(entry.message(), entry.channelEntity().getId());
|
||||||
ReentrantLock lock = sessionLocks.computeIfAbsent(conversationId, k -> new ReentrantLock());
|
ReentrantLock lock = sessionLocks.computeIfAbsent(conversationId, k -> new ReentrantLock());
|
||||||
|
|
||||||
lock.lock();
|
lock.lock();
|
||||||
@ -1487,7 +1487,7 @@ public class ChannelMessageRouter {
|
|||||||
return Flux.error(new IllegalStateException("Channel has no associated agent"));
|
return Flux.error(new IllegalStateException("Channel has no associated agent"));
|
||||||
}
|
}
|
||||||
|
|
||||||
String conversationId = buildConversationId(message);
|
String conversationId = buildConversationId(message, channelEntity.getId());
|
||||||
String username = message.getSenderName() != null ? message.getSenderName() : message.getSenderId();
|
String username = message.getSenderName() != null ? message.getSenderName() : message.getSenderId();
|
||||||
|
|
||||||
conversationService.getOrCreateConversation(conversationId, agentId, username, channelEntity.getWorkspaceId());
|
conversationService.getOrCreateConversation(conversationId, agentId, username, channelEntity.getWorkspaceId());
|
||||||
@ -1604,9 +1604,26 @@ public class ChannelMessageRouter {
|
|||||||
* 格式:{channelType}:{chatId 或 senderId}
|
* 格式:{channelType}:{chatId 或 senderId}
|
||||||
* 格式采用 {channelType}:{identifier} 命名规则
|
* 格式采用 {channelType}:{identifier} 命名规则
|
||||||
*/
|
*/
|
||||||
private String buildConversationId(ChannelMessage message) {
|
/**
|
||||||
|
* Build the conversation id for an inbound channel message.
|
||||||
|
*
|
||||||
|
* <p>The id is scoped by {@code channelId} so the same sender reaching two
|
||||||
|
* different workspaces' same-type channels (e.g. two separate wecom channels)
|
||||||
|
* no longer collapses into one shared conversation row. {@code channelId} is
|
||||||
|
* the {@code ChannelEntity} primary key, which binds to exactly one workspace.
|
||||||
|
*
|
||||||
|
* <p>Format: {@code {channelType}:{channelId}:{chatId|senderId}}. When
|
||||||
|
* {@code channelId} is null (defensive; the routed channel row always has an
|
||||||
|
* id) the legacy {@code {channelType}:{identifier}} form is used so nothing
|
||||||
|
* NPEs — those ids remain workspace-ambiguous but that path is not reachable
|
||||||
|
* for a persisted channel.
|
||||||
|
*/
|
||||||
|
private String buildConversationId(ChannelMessage message, Long channelId) {
|
||||||
String identifier = message.getChatId() != null ? message.getChatId() : message.getSenderId();
|
String identifier = message.getChatId() != null ? message.getChatId() : message.getSenderId();
|
||||||
return message.getChannelType() + ":" + identifier;
|
if (channelId == null) {
|
||||||
|
return message.getChannelType() + ":" + identifier;
|
||||||
|
}
|
||||||
|
return message.getChannelType() + ":" + channelId + ":" + identifier;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@ -1212,7 +1212,8 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre
|
|||||||
if (isGroup && chatId != null) {
|
if (isGroup && chatId != null) {
|
||||||
shortSuffix = resolveGroupSessionSuffix(chatId);
|
shortSuffix = resolveGroupSessionSuffix(chatId);
|
||||||
}
|
}
|
||||||
String conversationId = buildConversationId(shortSuffix, senderOpenId, isGroup);
|
String conversationId = buildConversationId(shortSuffix, senderOpenId, isGroup,
|
||||||
|
channelEntity != null ? channelEntity.getId() : null);
|
||||||
|
|
||||||
String stagedUploadPath = null;
|
String stagedUploadPath = null;
|
||||||
if (isFileMessage) {
|
if (isFileMessage) {
|
||||||
@ -1731,14 +1732,22 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre
|
|||||||
* {@code senderId} is the full open id. Mirror that exactly:
|
* {@code senderId} is the full open id. Mirror that exactly:
|
||||||
* {@code groups → feishu:{shortSuffix}}, {@code DMs → feishu:{senderOpenId}}.
|
* {@code groups → feishu:{shortSuffix}}, {@code DMs → feishu:{senderOpenId}}.
|
||||||
*/
|
*/
|
||||||
static String buildConversationId(String shortSuffix, String senderOpenId, boolean isGroup) {
|
static String buildConversationId(String shortSuffix, String senderOpenId, boolean isGroup,
|
||||||
|
Long channelId) {
|
||||||
// The routed ChannelMessage carries chatId = (isGroup ? shortSuffix : null);
|
// The routed ChannelMessage carries chatId = (isGroup ? shortSuffix : null);
|
||||||
// the router then falls back to senderId when that chatId is null. Mirror both
|
// the router then falls back to senderId when that chatId is null. Mirror both
|
||||||
// steps so the storage id matches the runtime id in every case (including the
|
// steps so the storage id matches the runtime id in every case (including the
|
||||||
// degenerate group-with-no-suffix path).
|
// degenerate group-with-no-suffix path).
|
||||||
String routedChatId = isGroup ? shortSuffix : null;
|
String routedChatId = isGroup ? shortSuffix : null;
|
||||||
String identifier = routedChatId != null ? routedChatId : senderOpenId;
|
String identifier = routedChatId != null ? routedChatId : senderOpenId;
|
||||||
return identifier != null ? CHANNEL_TYPE + ":" + identifier : null;
|
if (identifier == null) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
// Mirror ChannelMessageRouter#buildConversationId: scope the id by channelId so
|
||||||
|
// the same sender on two workspaces' feishu channels never shares a conversation.
|
||||||
|
return channelId != null
|
||||||
|
? CHANNEL_TYPE + ":" + channelId + ":" + identifier
|
||||||
|
: CHANNEL_TYPE + ":" + identifier;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ==================== Per-chat recent file cache ====================
|
// ==================== Per-chat recent file cache ====================
|
||||||
|
|||||||
@ -320,15 +320,22 @@ public class ToolGuardCardHandler implements FeishuCardHandler {
|
|||||||
private static ChannelMessage buildSynthetic(String commandText, String clickerOpenId,
|
private static ChannelMessage buildSynthetic(String commandText, String clickerOpenId,
|
||||||
PendingApproval pending,
|
PendingApproval pending,
|
||||||
P2CardActionTriggerData data) {
|
P2CardActionTriggerData data) {
|
||||||
// pending.conversationId looks like "feishu:<scope>" where
|
// pending.conversationId looks like "feishu:{channelId}:{scope}" (or the
|
||||||
// <scope> is either ou_xxx (1:1 chat — derived from senderId)
|
// legacy two-segment "feishu:{scope}"), where {scope} is either ou_xxx
|
||||||
// or oc_xxx (group chat — derived from chatId). Reverse the
|
// (1:1 chat — derived from senderId) or oc_xxx (group chat — derived from
|
||||||
// scope back into the right chatId field so buildConversationId
|
// chatId). Extract the trailing {scope} and reverse it back into the right
|
||||||
// reproduces the exact same key.
|
// chatId field so buildConversationId reproduces the exact same key. The
|
||||||
|
// replay routes through this same feishu channel, so the router re-embeds
|
||||||
|
// the matching channelId automatically. Scopes never contain ':', so
|
||||||
|
// splitting on the first ':' after the "feishu:" prefix is unambiguous and
|
||||||
|
// handles both the new three-segment and the legacy two-segment forms.
|
||||||
String convId = pending.getConversationId();
|
String convId = pending.getConversationId();
|
||||||
String scope = (convId != null && convId.startsWith("feishu:"))
|
String scope = null;
|
||||||
? convId.substring("feishu:".length())
|
if (convId != null && convId.startsWith("feishu:")) {
|
||||||
: null;
|
String rest = convId.substring("feishu:".length());
|
||||||
|
int colon = rest.indexOf(':');
|
||||||
|
scope = colon >= 0 ? rest.substring(colon + 1) : rest;
|
||||||
|
}
|
||||||
boolean isGroup = scope != null && scope.startsWith("oc_");
|
boolean isGroup = scope != null && scope.startsWith("oc_");
|
||||||
String chatId = isGroup ? scope : null;
|
String chatId = isGroup ? scope : null;
|
||||||
String replyToken = isGroup ? scope : clickerOpenId;
|
String replyToken = isGroup ? scope : clickerOpenId;
|
||||||
|
|||||||
@ -999,7 +999,7 @@ public class WeComChannelAdapter extends AbstractChannelAdapter implements Strea
|
|||||||
Map<String, Object> imgBody = (Map<String, Object>) body.getOrDefault("image", Map.of());
|
Map<String, Object> imgBody = (Map<String, Object>) body.getOrDefault("image", Map.of());
|
||||||
String url = (String) imgBody.getOrDefault("url", "");
|
String url = (String) imgBody.getOrDefault("url", "");
|
||||||
String aesKey = (String) imgBody.getOrDefault("aeskey", "");
|
String aesKey = (String) imgBody.getOrDefault("aeskey", "");
|
||||||
String inboundConvId = inboundConversationId(senderId, chatId, chatType);
|
String inboundConvId = inboundConversationId(senderId, chatId, chatType, channelEntity != null ? channelEntity.getId() : null);
|
||||||
if (!url.isBlank()) {
|
if (!url.isBlank()) {
|
||||||
contentParts.add(buildInboundImagePart(url, aesKey, msgId, "image.jpg", inboundConvId));
|
contentParts.add(buildInboundImagePart(url, aesKey, msgId, "image.jpg", inboundConvId));
|
||||||
}
|
}
|
||||||
@ -1028,7 +1028,7 @@ public class WeComChannelAdapter extends AbstractChannelAdapter implements Strea
|
|||||||
String filename = (String) fileBody.getOrDefault("filename",
|
String filename = (String) fileBody.getOrDefault("filename",
|
||||||
fileBody.getOrDefault("file_name",
|
fileBody.getOrDefault("file_name",
|
||||||
fileBody.getOrDefault("name", "file.bin")));
|
fileBody.getOrDefault("name", "file.bin")));
|
||||||
String fileConvId = inboundConversationId(senderId, chatId, chatType);
|
String fileConvId = inboundConversationId(senderId, chatId, chatType, channelEntity != null ? channelEntity.getId() : null);
|
||||||
if (!url.isBlank()) {
|
if (!url.isBlank()) {
|
||||||
MessageContentPart filePart = buildInboundFilePart(
|
MessageContentPart filePart = buildInboundFilePart(
|
||||||
url, aesKey, msgId, filename, fileConvId);
|
url, aesKey, msgId, filename, fileConvId);
|
||||||
@ -1058,7 +1058,7 @@ public class WeComChannelAdapter extends AbstractChannelAdapter implements Strea
|
|||||||
Map<String, Object> img = (Map<String, Object>) item.getOrDefault("image", Map.of());
|
Map<String, Object> img = (Map<String, Object>) item.getOrDefault("image", Map.of());
|
||||||
String url = (String) img.getOrDefault("url", "");
|
String url = (String) img.getOrDefault("url", "");
|
||||||
String aesKey = (String) img.getOrDefault("aeskey", "");
|
String aesKey = (String) img.getOrDefault("aeskey", "");
|
||||||
String mixedConvId = inboundConversationId(senderId, chatId, chatType);
|
String mixedConvId = inboundConversationId(senderId, chatId, chatType, channelEntity != null ? channelEntity.getId() : null);
|
||||||
if (!url.isBlank()) {
|
if (!url.isBlank()) {
|
||||||
contentParts.add(buildInboundImagePart(
|
contentParts.add(buildInboundImagePart(
|
||||||
url, aesKey, msgId, "mixed_image.jpg", mixedConvId));
|
url, aesKey, msgId, "mixed_image.jpg", mixedConvId));
|
||||||
@ -2936,9 +2936,15 @@ public class WeComChannelAdapter extends AbstractChannelAdapter implements Strea
|
|||||||
* matching row, returned 403, and the IM client rendered every
|
* matching row, returned 403, and the IM client rendered every
|
||||||
* group-quoted image as a broken icon.
|
* group-quoted image as a broken icon.
|
||||||
*/
|
*/
|
||||||
private static String inboundConversationId(String senderId, String chatId, String chatType) {
|
private static String inboundConversationId(String senderId, String chatId, String chatType,
|
||||||
|
Long channelId) {
|
||||||
boolean isGroup = "group".equals(chatType);
|
boolean isGroup = "group".equals(chatType);
|
||||||
return isGroup ? "wecom:" + chatId : "wecom:" + senderId;
|
String identifier = isGroup ? chatId : senderId;
|
||||||
|
// Mirror ChannelMessageRouter#buildConversationId: scope the id by channelId so the
|
||||||
|
// same sender on two workspaces' wecom channels never shares a conversation. channelId
|
||||||
|
// is the ChannelEntity primary key; a null (unreachable for a persisted channel) keeps
|
||||||
|
// the legacy two-segment form so nothing NPEs.
|
||||||
|
return channelId != null ? "wecom:" + channelId + ":" + identifier : "wecom:" + identifier;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ==================== 引用消息(quote)解析 ====================
|
// ==================== 引用消息(quote)解析 ====================
|
||||||
@ -2994,7 +3000,7 @@ public class WeComChannelAdapter extends AbstractChannelAdapter implements Strea
|
|||||||
items = List.of(quote);
|
items = List.of(quote);
|
||||||
}
|
}
|
||||||
|
|
||||||
String inboundConvId = inboundConversationId(senderId, chatId, chatType);
|
String inboundConvId = inboundConversationId(senderId, chatId, chatType, channelEntity != null ? channelEntity.getId() : null);
|
||||||
StringBuilder summary = new StringBuilder();
|
StringBuilder summary = new StringBuilder();
|
||||||
List<MessageContentPart> attached = new ArrayList<>();
|
List<MessageContentPart> attached = new ArrayList<>();
|
||||||
|
|
||||||
@ -3106,7 +3112,7 @@ public class WeComChannelAdapter extends AbstractChannelAdapter implements Strea
|
|||||||
String title = ((String) appmsg.getOrDefault("title", "")).trim();
|
String title = ((String) appmsg.getOrDefault("title", "")).trim();
|
||||||
String desc = ((String) appmsg.getOrDefault("description", "")).trim();
|
String desc = ((String) appmsg.getOrDefault("description", "")).trim();
|
||||||
String linkUrl = ((String) appmsg.getOrDefault("url", "")).trim();
|
String linkUrl = ((String) appmsg.getOrDefault("url", "")).trim();
|
||||||
String inboundConvId = inboundConversationId(senderId, chatId, chatType);
|
String inboundConvId = inboundConversationId(senderId, chatId, chatType, channelEntity != null ? channelEntity.getId() : null);
|
||||||
|
|
||||||
Object fileObj = appmsg.get("file");
|
Object fileObj = appmsg.get("file");
|
||||||
Object imageObj = appmsg.get("image");
|
Object imageObj = appmsg.get("image");
|
||||||
|
|||||||
@ -347,8 +347,22 @@ public class ConversationService {
|
|||||||
conv.setMessageCount(0);
|
conv.setMessageCount(0);
|
||||||
conv.setLastActiveTime(LocalDateTime.now());
|
conv.setLastActiveTime(LocalDateTime.now());
|
||||||
conversationMapper.insert(conv);
|
conversationMapper.insert(conv);
|
||||||
} else if (!conv.getUsername().equals(username)) {
|
} else {
|
||||||
throw new IllegalArgumentException("无权操作该会话");
|
// Defense-in-depth workspace isolation: an existing row whose owning
|
||||||
|
// workspace differs from the caller's means two workspaces resolved to
|
||||||
|
// the same conversationId. With channel-scoped ids this should no longer
|
||||||
|
// happen for channel traffic; refuse rather than silently write the
|
||||||
|
// caller's message into another workspace's conversation. Also closes the
|
||||||
|
// web-console bare-"default" cross-workspace edge case.
|
||||||
|
if (workspaceId != null && conv.getWorkspaceId() != null
|
||||||
|
&& !conv.getWorkspaceId().equals(workspaceId)) {
|
||||||
|
log.warn("[Conversation] Cross-workspace conversationId collision: id={} owner={} requested={}",
|
||||||
|
conversationId, conv.getWorkspaceId(), workspaceId);
|
||||||
|
throw new IllegalArgumentException("会话不属于当前工作区");
|
||||||
|
}
|
||||||
|
if (!conv.getUsername().equals(username)) {
|
||||||
|
throw new IllegalArgumentException("无权操作该会话");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return conv;
|
return conv;
|
||||||
}
|
}
|
||||||
|
|||||||
@ -0,0 +1,9 @@
|
|||||||
|
-- V171: widen conversation_id to hold the channel-scoped id format
|
||||||
|
-- (H2 dialect). Channel conversation ids now carry a channelId segment
|
||||||
|
-- ({channelType}:{channelId}:{sender|chat}), which can exceed the old
|
||||||
|
-- VARCHAR(64). Widen the two strongly-bound tables to VARCHAR(128), matching
|
||||||
|
-- mate_channel_session / audit tables. The UNIQUE index on
|
||||||
|
-- mate_conversation.conversation_id is preserved by the type change.
|
||||||
|
|
||||||
|
ALTER TABLE mate_conversation ALTER COLUMN conversation_id SET DATA TYPE VARCHAR(128);
|
||||||
|
ALTER TABLE mate_message ALTER COLUMN conversation_id SET DATA TYPE VARCHAR(128);
|
||||||
@ -0,0 +1,6 @@
|
|||||||
|
-- See the H2 file for context. KingbaseES (PostgreSQL-compatible) uses
|
||||||
|
-- ALTER COLUMN ... TYPE. The NOT NULL and UNIQUE constraints on the column are
|
||||||
|
-- preserved by a type change.
|
||||||
|
|
||||||
|
ALTER TABLE mate_conversation ALTER COLUMN conversation_id TYPE VARCHAR(128);
|
||||||
|
ALTER TABLE mate_message ALTER COLUMN conversation_id TYPE VARCHAR(128);
|
||||||
@ -0,0 +1,8 @@
|
|||||||
|
-- See the H2 file for context. MySQL widens with MODIFY COLUMN; the full column
|
||||||
|
-- definition is restated so NOT NULL is preserved (MODIFY replaces the whole
|
||||||
|
-- definition). Widening the type keeps the existing UNIQUE index on
|
||||||
|
-- mate_conversation.conversation_id. Widening is idempotent enough that Flyway's
|
||||||
|
-- version tracking is the only re-run guard needed (no ADD COLUMN existence check).
|
||||||
|
|
||||||
|
ALTER TABLE mate_conversation MODIFY COLUMN conversation_id VARCHAR(128) NOT NULL;
|
||||||
|
ALTER TABLE mate_message MODIFY COLUMN conversation_id VARCHAR(128) NOT NULL;
|
||||||
@ -18,57 +18,74 @@ import static org.junit.jupiter.api.Assertions.*;
|
|||||||
*
|
*
|
||||||
* <p>The router derives its id from the routed {@code ChannelMessage}, whose
|
* <p>The router derives its id from the routed {@code ChannelMessage}, whose
|
||||||
* {@code chatId} is {@code (isGroup ? shortSuffix : null)} and whose {@code senderId}
|
* {@code chatId} is {@code (isGroup ? shortSuffix : null)} and whose {@code senderId}
|
||||||
* is the full open id, via {@code feishu:{chatId != null ? chatId : senderId}}.
|
* is the full open id, via {@code feishu:{channelId}:{chatId != null ? chatId : senderId}}.
|
||||||
|
* The {@code channelId} segment scopes the id to one channel row (hence one workspace)
|
||||||
|
* so the same sender on two workspaces' feishu channels never collides into one id.
|
||||||
*/
|
*/
|
||||||
class FeishuConversationIdAlignmentTest {
|
class FeishuConversationIdAlignmentTest {
|
||||||
|
|
||||||
private static final String CHANNEL = FeishuChannelAdapter.CHANNEL_TYPE; // "feishu"
|
private static final String CHANNEL = FeishuChannelAdapter.CHANNEL_TYPE; // "feishu"
|
||||||
private static final String SHORT_SUFFIX = "cli2_abcd1234";
|
private static final String SHORT_SUFFIX = "cli2_abcd1234";
|
||||||
private static final String SENDER = "ou_user0123456789";
|
private static final String SENDER = "ou_user0123456789";
|
||||||
|
private static final Long CHANNEL_ID = 2056987497408438273L;
|
||||||
|
|
||||||
/** Mirror of ChannelMessageRouter#buildConversationId against the routed message. */
|
/** Mirror of ChannelMessageRouter#buildConversationId against the routed message. */
|
||||||
private static String routerConversationId(String shortSuffix, String senderId, boolean isGroup) {
|
private static String routerConversationId(String shortSuffix, String senderId, boolean isGroup,
|
||||||
|
Long channelId) {
|
||||||
String routedChatId = isGroup ? shortSuffix : null;
|
String routedChatId = isGroup ? shortSuffix : null;
|
||||||
String identifier = routedChatId != null ? routedChatId : senderId;
|
String identifier = routedChatId != null ? routedChatId : senderId;
|
||||||
return identifier != null ? CHANNEL + ":" + identifier : null;
|
if (identifier == null) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return channelId != null
|
||||||
|
? CHANNEL + ":" + channelId + ":" + identifier
|
||||||
|
: CHANNEL + ":" + identifier;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void group_usesShortSuffix() {
|
void group_usesShortSuffix() {
|
||||||
assertEquals(CHANNEL + ":" + SHORT_SUFFIX,
|
assertEquals(CHANNEL + ":" + CHANNEL_ID + ":" + SHORT_SUFFIX,
|
||||||
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, true));
|
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, true, CHANNEL_ID));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void dm_usesSenderOpenIdAndIgnoresShortSuffix() {
|
void dm_usesSenderOpenIdAndIgnoresShortSuffix() {
|
||||||
assertEquals(CHANNEL + ":" + SENDER,
|
assertEquals(CHANNEL + ":" + CHANNEL_ID + ":" + SENDER,
|
||||||
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false));
|
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false, CHANNEL_ID));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void group_nullShortSuffix_fallsBackToSender() {
|
void group_nullShortSuffix_fallsBackToSender() {
|
||||||
// Degenerate group path: routed chatId is null, so the router (and this helper)
|
// Degenerate group path: routed chatId is null, so the router (and this helper)
|
||||||
// fall back to the sender open id — never null when a sender is present.
|
// fall back to the sender open id — never null when a sender is present.
|
||||||
assertEquals(CHANNEL + ":" + SENDER,
|
assertEquals(CHANNEL + ":" + CHANNEL_ID + ":" + SENDER,
|
||||||
FeishuChannelAdapter.buildConversationId(null, SENDER, true));
|
FeishuChannelAdapter.buildConversationId(null, SENDER, true, CHANNEL_ID));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void dm_nullSender_returnsNull() {
|
void dm_nullSender_returnsNull() {
|
||||||
assertNull(FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, null, false));
|
assertNull(FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, null, false, CHANNEL_ID));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void nullChannelId_fallsBackToLegacyTwoSegmentForm() {
|
||||||
|
// Defensive: a null channelId (unreachable for a persisted channel row) keeps
|
||||||
|
// the legacy {channelType}:{identifier} form so nothing NPEs.
|
||||||
|
assertEquals(CHANNEL + ":" + SENDER,
|
||||||
|
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false, null));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void matchesRouterFormula_acrossCases() {
|
void matchesRouterFormula_acrossCases() {
|
||||||
// Group and DM, with and without a short suffix — the storage id must equal
|
// Group and DM, with and without a short suffix — the storage id must equal
|
||||||
// the id the router computes for the routed ChannelMessage in every case.
|
// the id the router computes for the routed ChannelMessage in every case.
|
||||||
assertEquals(routerConversationId(SHORT_SUFFIX, SENDER, true),
|
assertEquals(routerConversationId(SHORT_SUFFIX, SENDER, true, CHANNEL_ID),
|
||||||
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, true));
|
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, true, CHANNEL_ID));
|
||||||
assertEquals(routerConversationId(SHORT_SUFFIX, SENDER, false),
|
assertEquals(routerConversationId(SHORT_SUFFIX, SENDER, false, CHANNEL_ID),
|
||||||
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false));
|
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false, CHANNEL_ID));
|
||||||
assertEquals(routerConversationId(null, SENDER, true),
|
assertEquals(routerConversationId(null, SENDER, true, CHANNEL_ID),
|
||||||
FeishuChannelAdapter.buildConversationId(null, SENDER, true));
|
FeishuChannelAdapter.buildConversationId(null, SENDER, true, CHANNEL_ID));
|
||||||
assertEquals(routerConversationId(SHORT_SUFFIX, null, false),
|
assertEquals(routerConversationId(SHORT_SUFFIX, null, false, CHANNEL_ID),
|
||||||
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, null, false));
|
FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, null, false, CHANNEL_ID));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -20,38 +20,45 @@ import static org.junit.jupiter.api.Assertions.*;
|
|||||||
* different conversationId — and the {@code /api/v1/chat/files/{convId}/...}
|
* different conversationId — and the {@code /api/v1/chat/files/{convId}/...}
|
||||||
* endpoint's owner check fails for every fetch (403 → broken images).
|
* endpoint's owner check fails for every fetch (403 → broken images).
|
||||||
*
|
*
|
||||||
* <p>The format both produce: {@code wecom:{chatId}} for groups,
|
* <p>The format both produce: {@code wecom:{channelId}:{chatId}} for groups,
|
||||||
* {@code wecom:{senderId}} for 1:1 — no {@code group:} infix.
|
* {@code wecom:{channelId}:{senderId}} for 1:1 — no {@code group:} infix. The
|
||||||
|
* {@code channelId} segment scopes the id to one channel row (hence one
|
||||||
|
* workspace) so the same sender on two workspaces' wecom channels never
|
||||||
|
* collides into one conversation.
|
||||||
*/
|
*/
|
||||||
class WeComInboundConversationIdTest {
|
class WeComInboundConversationIdTest {
|
||||||
|
|
||||||
|
private static final Long CHANNEL_ID = 2056987497408438273L;
|
||||||
|
|
||||||
private static String inboundConversationId(String senderId, String chatId, String chatType) throws Exception {
|
private static String inboundConversationId(String senderId, String chatId, String chatType) throws Exception {
|
||||||
Method m = WeComChannelAdapter.class.getDeclaredMethod(
|
Method m = WeComChannelAdapter.class.getDeclaredMethod(
|
||||||
"inboundConversationId", String.class, String.class, String.class);
|
"inboundConversationId", String.class, String.class, String.class, Long.class);
|
||||||
m.setAccessible(true);
|
m.setAccessible(true);
|
||||||
return (String) m.invoke(null, senderId, chatId, chatType);
|
return (String) m.invoke(null, senderId, chatId, chatType, CHANNEL_ID);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Mirror of ChannelMessageRouter#buildConversationId for cross-checking. */
|
||||||
|
private static String routerConversationId(String identifier) {
|
||||||
|
return "wecom:" + CHANNEL_ID + ":" + identifier;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@DisplayName("group → wecom:{chatId} (no 'group:' infix, matches router)")
|
@DisplayName("group → wecom:{channelId}:{chatId} (no 'group:' infix, matches router)")
|
||||||
void groupChatIdFormat() throws Exception {
|
void groupChatIdFormat() throws Exception {
|
||||||
// The bug fix: previously returned "wecom:group:abc" which mismatched
|
// The channelId segment scopes the id to one channel/workspace; the
|
||||||
// the router's "wecom:abc" — quoted-image fileUrls hit a 403 because
|
// group branch still uses chatId (no "group:" infix) to match the router.
|
||||||
// isConversationOwner couldn't find a "wecom:group:abc" row in
|
assertEquals("wecom:" + CHANNEL_ID + ":group-abc",
|
||||||
// mate_conversation.
|
|
||||||
assertEquals("wecom:group-abc",
|
|
||||||
inboundConversationId("XuZhanFu", "group-abc", "group"));
|
inboundConversationId("XuZhanFu", "group-abc", "group"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@DisplayName("1:1 → wecom:{senderId} (chatId is irrelevant in single chats)")
|
@DisplayName("1:1 → wecom:{channelId}:{senderId} (chatId is irrelevant in single chats)")
|
||||||
void singleChatSenderFormat() throws Exception {
|
void singleChatSenderFormat() throws Exception {
|
||||||
// Single-chat case never had the bug because both adapter and
|
// Single-chat case: both adapter and router fall back to senderId; pin it
|
||||||
// router fell back to senderId — pin it so a future refactor of
|
// so a future refactor of either side doesn't accidentally diverge.
|
||||||
// either side doesn't accidentally diverge.
|
assertEquals("wecom:" + CHANNEL_ID + ":XuZhanFu",
|
||||||
assertEquals("wecom:XuZhanFu",
|
|
||||||
inboundConversationId("XuZhanFu", null, "single"));
|
inboundConversationId("XuZhanFu", null, "single"));
|
||||||
assertEquals("wecom:XuZhanFu",
|
assertEquals("wecom:" + CHANNEL_ID + ":XuZhanFu",
|
||||||
inboundConversationId("XuZhanFu", "ignored-when-single", "single"));
|
inboundConversationId("XuZhanFu", "ignored-when-single", "single"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -59,19 +66,13 @@ class WeComInboundConversationIdTest {
|
|||||||
@DisplayName("matches ChannelMessageRouter.buildConversationId for both group and 1:1")
|
@DisplayName("matches ChannelMessageRouter.buildConversationId for both group and 1:1")
|
||||||
void matchesRouterFormat() throws Exception {
|
void matchesRouterFormat() throws Exception {
|
||||||
// Router's identifier picker:
|
// Router's identifier picker:
|
||||||
// chatId != null → "{channelType}:{chatId}" (group)
|
// chatId != null → "{channelType}:{channelId}:{chatId}" (group)
|
||||||
// chatId == null → "{channelType}:{senderId}" (single)
|
// chatId == null → "{channelType}:{channelId}:{senderId}" (single)
|
||||||
// Inbound side passes chatId for groups, null/ignored for 1:1.
|
// Inbound side passes chatId for groups, null/ignored for 1:1.
|
||||||
// Both must arrive at the same string, exact-equal.
|
// Both must arrive at the same string, exact-equal.
|
||||||
|
assertEquals(routerConversationId("group-xyz"),
|
||||||
// group: router gets chatId from the ChannelMessage builder
|
|
||||||
String routerGroup = "wecom" + ":" + "group-xyz";
|
|
||||||
assertEquals(routerGroup,
|
|
||||||
inboundConversationId("Alice", "group-xyz", "group"));
|
inboundConversationId("Alice", "group-xyz", "group"));
|
||||||
|
assertEquals(routerConversationId("Alice"),
|
||||||
// single: router falls back to senderId (chatId is null on the message)
|
|
||||||
String routerSingle = "wecom" + ":" + "Alice";
|
|
||||||
assertEquals(routerSingle,
|
|
||||||
inboundConversationId("Alice", null, "single"));
|
inboundConversationId("Alice", null, "single"));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -22,6 +22,7 @@ import vip.mate.workspace.conversation.repository.MessageMapper;
|
|||||||
import vip.mate.workspace.core.service.WorkspaceService;
|
import vip.mate.workspace.core.service.WorkspaceService;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||||
import static org.mockito.ArgumentMatchers.any;
|
import static org.mockito.ArgumentMatchers.any;
|
||||||
import static org.mockito.ArgumentMatchers.anyLong;
|
import static org.mockito.ArgumentMatchers.anyLong;
|
||||||
import static org.mockito.ArgumentMatchers.anyString;
|
import static org.mockito.ArgumentMatchers.anyString;
|
||||||
@ -240,6 +241,38 @@ class ConversationServiceOwnershipWorkspaceTest {
|
|||||||
return u;
|
return u;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ------------------------------------------------------------------
|
||||||
|
// getOrCreateConversation cross-workspace defense (channel-scoping)
|
||||||
|
// ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("getOrCreate: existing row in another workspace → rejected, no insert")
|
||||||
|
void getOrCreateCrossWorkspaceRejected() {
|
||||||
|
// An existing conversation owned by workspace A; a caller from workspace B
|
||||||
|
// resolving the same id must be refused rather than silently writing into A.
|
||||||
|
when(conversationMapper.selectOne(any()))
|
||||||
|
.thenReturn(conv(ALICE_CONV, "alice", WS_TENANT_A));
|
||||||
|
|
||||||
|
assertThatThrownBy(() ->
|
||||||
|
service.getOrCreateConversation(ALICE_CONV, 1L, "alice", WS_TENANT_B))
|
||||||
|
.isInstanceOf(IllegalArgumentException.class)
|
||||||
|
.hasMessageContaining("工作区");
|
||||||
|
verify(conversationMapper, never()).insert(any(ConversationEntity.class));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("getOrCreate: existing row in the same workspace → returned, no throw")
|
||||||
|
void getOrCreateSameWorkspaceReturns() {
|
||||||
|
when(conversationMapper.selectOne(any()))
|
||||||
|
.thenReturn(conv(ALICE_CONV, "alice", WS_TENANT_A));
|
||||||
|
|
||||||
|
ConversationEntity got =
|
||||||
|
service.getOrCreateConversation(ALICE_CONV, 1L, "alice", WS_TENANT_A);
|
||||||
|
|
||||||
|
assertThat(got.getConversationId()).isEqualTo(ALICE_CONV);
|
||||||
|
verify(conversationMapper, never()).insert(any(ConversationEntity.class));
|
||||||
|
}
|
||||||
|
|
||||||
private static ConversationEntity conv(String conversationId, String username, Long workspaceId) {
|
private static ConversationEntity conv(String conversationId, String username, Long workspaceId) {
|
||||||
ConversationEntity c = new ConversationEntity();
|
ConversationEntity c = new ConversationEntity();
|
||||||
c.setConversationId(conversationId);
|
c.setConversationId(conversationId);
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user