mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-13 00:14:33 +00:00
feat(bitrix24): send pairing reply with code instead of silent drop [B24:2794] (#1445)
When an agent's DM/Group policy is "pairing (require code)", an unpaired Bitrix24 sender got silence: the channel only had a Phase 03 stub (logPairingNeeded) that logged "pairing required (Phase 07 will send pairing reply)" and dropped the message — no code was ever generated or sent, so an admin had nothing to approve. Bitrix24 was the only channel missing this; Discord/Zalo/WhatsApp/Slack/Feishu already send a pairing reply. Replace the stub with sendPairingReply, mirroring the other channels: - RequestPairing() to generate the code (keyed by senderID for DMs, "group:<chatID>" for groups so it matches CheckGroupPolicy.IsPaired). - Render the shared systemmessages.KeyPairingAccountRequired message (code + how to approve). - Send it back into the chat via the v2 API (imbot.v2.Chat.Message.send), consistent with the welcome / OAuth-invite messages — not the legacy imbot.message.add v1. - Keep the per-sender debounce so a spammy sender can't flood the chat. Tests: send fires on the v2 endpoint with the code, no-op without a pairing service, and the second call within the debounce window is suppressed. Co-authored-by: DangTinh311 <dangtinh31193@gmail.com>
This commit is contained in:
1 parent
aa27354d16
commit
6cf48a6b08
2 files changed
+165
-16
No files matched your search
@@ -13,6 +13,7 @@ import (
|
||||
"github.com/nextlevelbuilder/goclaw/internal/channels"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/channels/media"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/systemmessages"
|
||||
)
|
||||
|
||||
// mentionMatcher is the compiled-once regex + rendered tag string used to
|
||||
@@ -91,8 +92,8 @@ func (c *Channel) DispatchEvent(ctx context.Context, evt *Event) {
|
||||
// 2. Group-only: require-mention gate + strip the mention from the text.
|
||||
// 3. Skip empty payloads (no text + no media).
|
||||
// 4. Policy: DM vs Group via BaseChannel.CheckDMPolicy / CheckGroupPolicy.
|
||||
// Pairing policies trigger a pairing-reply stub (Phase 07 will wire up
|
||||
// the full pairing flow; Phase 03 logs and drops).
|
||||
// Pairing policies send a pairing reply (code + approve instructions) via
|
||||
// sendPairingReply; deny drops silently.
|
||||
// 5. Build metadata map with bitrix_* keys so the agent can echo / reply
|
||||
// to the right dialog + message ID later.
|
||||
// 6. Forward to BaseChannel.HandleMessage → publishes bus.InboundMessage.
|
||||
@@ -258,16 +259,17 @@ func (c *Channel) handleMessage(ctx context.Context, evt *Event) {
|
||||
peerKind = "group"
|
||||
}
|
||||
|
||||
// Policy gating. BaseChannel reports PolicyNeedsPairing for unpaired DMs
|
||||
// under the default "pairing" policy. Phase 07 will answer with a real
|
||||
// pairing reply; Phase 03 logs the intent so the flow is visible while
|
||||
// the rest of the channel comes online.
|
||||
// Policy gating. BaseChannel reports PolicyNeedsPairing for unpaired senders
|
||||
// under the "pairing" policy. Answer with a real pairing reply (code + how to
|
||||
// approve) via the v2 chat API, matching the other channels (Discord/Zalo/
|
||||
// WhatsApp/Slack/Feishu). Group pairings are keyed by "group:<chatID>" so the
|
||||
// generated code matches what CheckGroupPolicy.IsPaired later verifies.
|
||||
if peerKind == "direct" {
|
||||
switch c.CheckDMPolicy(ctx, senderID, c.cfg.DMPolicy) {
|
||||
case channels.PolicyDeny:
|
||||
return
|
||||
case channels.PolicyNeedsPairing:
|
||||
c.logPairingNeeded(senderID, chatID, peerKind)
|
||||
c.sendPairingReply(ctx, senderID, chatID, peerKind)
|
||||
return
|
||||
}
|
||||
} else {
|
||||
@@ -275,7 +277,7 @@ func (c *Channel) handleMessage(ctx context.Context, evt *Event) {
|
||||
case channels.PolicyDeny:
|
||||
return
|
||||
case channels.PolicyNeedsPairing:
|
||||
c.logPairingNeeded(senderID, chatID, peerKind)
|
||||
c.sendPairingReply(ctx, "group:"+chatID, chatID, peerKind)
|
||||
return
|
||||
}
|
||||
}
|
||||
@@ -597,16 +599,45 @@ func (c *Channel) mention() *mentionMatcher {
|
||||
return c.mentionRe
|
||||
}
|
||||
|
||||
// logPairingNeeded is the Phase 03 stand-in for a real pairing reply.
|
||||
// Phase 07 replaces this with a proper pairing message via imbot.message.add;
|
||||
// until then we log the intent so operators can see unpaired attempts.
|
||||
func (c *Channel) logPairingNeeded(senderID, chatID, peerKind string) {
|
||||
if !c.CanSendPairingNotif(senderID, pairingDebounce) {
|
||||
// sendPairingReply requests a pairing code for an unpaired sender and sends it
|
||||
// back into the chat via the v2 API (imbot.v2.Chat.Message.send) so an admin can
|
||||
// approve access. Mirrors the pairing reply the other channels already send
|
||||
// (Discord/Zalo/WhatsApp/Slack/Feishu) using the shared pairing.account_required
|
||||
// system message. Debounced per-sender so a spammy sender can't flood the chat.
|
||||
//
|
||||
// pairingSenderID is the id the pairing is keyed by — the raw senderID for DMs,
|
||||
// "group:<chatID>" for groups — so the generated code matches what CheckDMPolicy
|
||||
// / CheckGroupPolicy later verify via IsPaired.
|
||||
func (c *Channel) sendPairingReply(ctx context.Context, pairingSenderID, chatID, peerKind string) {
|
||||
ps := c.PairingService()
|
||||
if ps == nil {
|
||||
return
|
||||
}
|
||||
c.MarkPairingNotifSent(senderID)
|
||||
slog.Info("bitrix24: pairing required (Phase 07 will send pairing reply)",
|
||||
"sender_id", senderID, "chat_id", chatID, "peer_kind", peerKind,
|
||||
if !c.CanSendPairingNotif(pairingSenderID, pairingDebounce) {
|
||||
return
|
||||
}
|
||||
|
||||
code, err := ps.RequestPairing(ctx, pairingSenderID, c.Name(), chatID, "default", nil)
|
||||
if err != nil {
|
||||
slog.Debug("bitrix24 pairing: request failed",
|
||||
"sender_id", pairingSenderID, "chat_id", chatID, "err", err)
|
||||
return
|
||||
}
|
||||
|
||||
replyText := c.SystemMessage("", systemmessages.KeyPairingAccountRequired, systemmessages.Vars{
|
||||
"platform": "Bitrix24",
|
||||
"sender_id": pairingSenderID,
|
||||
"code": code,
|
||||
})
|
||||
|
||||
if err := c.sendChunkV2Public(ctx, chatID, replyText, 0); err != nil {
|
||||
slog.Warn("bitrix24 pairing: reply send failed",
|
||||
"sender_id", pairingSenderID, "chat_id", chatID, "err", err)
|
||||
return
|
||||
}
|
||||
c.MarkPairingNotifSent(pairingSenderID)
|
||||
slog.Info("bitrix24 pairing: reply sent",
|
||||
"sender_id", pairingSenderID, "chat_id", chatID, "peer_kind", peerKind,
|
||||
"portal", c.cfg.Portal)
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
package bitrix24
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/systemmessages"
|
||||
)
|
||||
|
||||
// fakeBitrixPairingStore implements store.PairingStore for pairing-reply tests.
|
||||
type fakeBitrixPairingStore struct {
|
||||
requested bool
|
||||
requestedFor string
|
||||
}
|
||||
|
||||
func (s *fakeBitrixPairingStore) RequestPairing(_ context.Context, senderID, _, _, _ string, _ map[string]string) (string, error) {
|
||||
s.requested = true
|
||||
s.requestedFor = senderID
|
||||
return "code123", nil
|
||||
}
|
||||
func (s *fakeBitrixPairingStore) ApprovePairing(context.Context, string, string) (*store.PairedDeviceData, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (s *fakeBitrixPairingStore) DenyPairing(context.Context, string) error { return nil }
|
||||
func (s *fakeBitrixPairingStore) RevokePairing(context.Context, string, string) error { return nil }
|
||||
func (s *fakeBitrixPairingStore) IsPaired(context.Context, string, string) (bool, error) {
|
||||
return false, nil
|
||||
}
|
||||
func (s *fakeBitrixPairingStore) ListPending(context.Context) []store.PairingRequestData { return nil }
|
||||
func (s *fakeBitrixPairingStore) ListPaired(context.Context) []store.PairedDeviceData { return nil }
|
||||
func (s *fakeBitrixPairingStore) MigrateGroupChatID(context.Context, string, string, string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// TestSendPairingReply_SendsCodeViaV2 verifies an unpaired sender gets a pairing
|
||||
// code sent back through the v2 chat API (imbot.v2.Chat.Message.send), not the
|
||||
// old v1 imbot.message.add, and not silence.
|
||||
func TestSendPairingReply_SendsCodeViaV2(t *testing.T) {
|
||||
var calls int32
|
||||
var gotPath, gotMessage string
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt32(&calls, 1)
|
||||
gotPath = r.URL.Path
|
||||
_ = r.ParseForm()
|
||||
gotMessage = r.Form.Get("fields[message]")
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"result": map[string]any{"id": 999}})
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
bc := newChannelWithBoundPortal(t, srv)
|
||||
bc.SetSystemMessages(systemmessages.NewResolver(nil))
|
||||
ps := &fakeBitrixPairingStore{}
|
||||
bc.SetPairingService(ps)
|
||||
|
||||
bc.sendPairingReply(context.Background(), "42", "chat100", "direct")
|
||||
|
||||
if atomic.LoadInt32(&calls) == 0 {
|
||||
t.Fatal("expected a REST send for the pairing reply, got none (bot stayed silent)")
|
||||
}
|
||||
if !strings.HasSuffix(gotPath, "/rest/imbot.v2.Chat.Message.send.json") {
|
||||
t.Errorf("pairing reply used wrong endpoint %q; want v2 imbot.v2.Chat.Message.send", gotPath)
|
||||
}
|
||||
if !ps.requested {
|
||||
t.Error("expected RequestPairing to be called to generate a code")
|
||||
}
|
||||
if !strings.Contains(gotMessage, "code123") {
|
||||
t.Errorf("pairing message should contain the code; got %q", gotMessage)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSendPairingReply_NoService_NoOp: without a pairing service the call is a
|
||||
// safe no-op (no send).
|
||||
func TestSendPairingReply_NoService_NoOp(t *testing.T) {
|
||||
var calls int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt32(&calls, 1)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"result": map[string]any{"id": 1}})
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
bc := newChannelWithBoundPortal(t, srv) // no SetPairingService → PairingService() nil
|
||||
bc.sendPairingReply(context.Background(), "42", "chat100", "direct")
|
||||
|
||||
if got := atomic.LoadInt32(&calls); got != 0 {
|
||||
t.Errorf("expected no send without a pairing service, got %d", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSendPairingReply_Debounced: a second call within the debounce window does
|
||||
// not fire a second send.
|
||||
func TestSendPairingReply_Debounced(t *testing.T) {
|
||||
var calls int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt32(&calls, 1)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"result": map[string]any{"id": 1}})
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
bc := newChannelWithBoundPortal(t, srv)
|
||||
bc.SetSystemMessages(systemmessages.NewResolver(nil))
|
||||
bc.SetPairingService(&fakeBitrixPairingStore{})
|
||||
|
||||
bc.sendPairingReply(context.Background(), "42", "chat100", "direct")
|
||||
bc.sendPairingReply(context.Background(), "42", "chat100", "direct") // debounced
|
||||
|
||||
if got := atomic.LoadInt32(&calls); got != 1 {
|
||||
t.Errorf("expected 1 send (second debounced), got %d", got)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user