fix(telegram): invert the chat gate — mute the ineligible (default-allow)
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 10s
CI / integration (pull_request) Successful in 19s
CI / ui (pull_request) Successful in 57s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m17s
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 10s
CI / integration (pull_request) Successful in 19s
CI / ui (pull_request) Successful in 57s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m17s
Telegram intersects the chat default with each user's permissions, so a per-user grant can never exceed a deny-by-default group: the original default-deny + grant design could not let any user write (can_send=true was AND-ed with the denying default). Invert it — the chat allows sending by default and the bot MUTES an ineligible member (unregistered, admin-suspended, or chat_muted) and restores an eligible one it had muted, acting only when the current state differs (idempotent, no self-loop). The block/unblock/chat_muted/registration path already sets can_send to the eligibility, so it is unchanged.
This commit is contained in:
@@ -3,7 +3,6 @@ package bot
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
@@ -77,13 +76,13 @@ func memberUpdate(chatID, userID int64, oldType models.ChatMemberType) *models.C
|
||||
}
|
||||
|
||||
// restrictedUpdate builds an oldType -> restricted transition for userID, the new
|
||||
// member's text-send permission set to canSend. A default-deny group reports a present
|
||||
// member as restricted with canSend=false.
|
||||
func restrictedUpdate(chatID, userID int64, oldType models.ChatMemberType, canSend bool) *models.ChatMemberUpdated {
|
||||
// member's text-send permission set to canSend and membership to isMember. A muted
|
||||
// member is restricted with canSend=false; an un-muted one with canSend=true.
|
||||
func restrictedUpdate(chatID, userID int64, oldType models.ChatMemberType, canSend, isMember bool) *models.ChatMemberUpdated {
|
||||
return &models.ChatMemberUpdated{
|
||||
Chat: models.Chat{ID: chatID},
|
||||
OldChatMember: models.ChatMember{Type: oldType, Left: &models.ChatMemberLeft{User: &models.User{ID: userID}}},
|
||||
NewChatMember: models.ChatMember{Type: models.ChatMemberTypeRestricted, Restricted: &models.ChatMemberRestricted{User: &models.User{ID: userID}, CanSendMessages: canSend}},
|
||||
NewChatMember: models.ChatMember{Type: models.ChatMemberTypeRestricted, Restricted: &models.ChatMemberRestricted{User: &models.User{ID: userID}, CanSendMessages: canSend, IsMember: isMember}},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -104,45 +103,73 @@ func eligibleBot(t *testing.T, api *chatAPI, eligible bool, err error) *Bot {
|
||||
return b
|
||||
}
|
||||
|
||||
func TestHandleChatMemberGrantsEligibleMemberJoin(t *testing.T) {
|
||||
func TestHandleChatMemberMutesIneligibleMember(t *testing.T) {
|
||||
// An unregistered/blocked member can send by the permissive default, so the bot mutes.
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
b := eligibleBot(t, api, false, nil)
|
||||
b.handleChatMember(context.Background(), memberUpdate(testChatID, 777, models.ChatMemberTypeLeft))
|
||||
if len(api.restricts) != 1 || api.restricts[0].userID != "777" || !api.restricts[0].canSend {
|
||||
t.Fatalf("restricts = %+v, want one grant (can_send=true) for 777", api.restricts)
|
||||
if len(api.restricts) != 1 || api.restricts[0].userID != "777" || api.restricts[0].canSend {
|
||||
t.Fatalf("restricts = %+v, want one mute (can_send=false) for 777", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberGrantsRestrictedMember(t *testing.T) {
|
||||
// A default-deny group reports a present member as restricted — the real-world case.
|
||||
// A fresh join (left->restricted) or an already-present restricted member
|
||||
// (restricted->restricted), and regardless of the reported can_send (which can be
|
||||
// misleading), an eligible member is granted.
|
||||
for _, tc := range []struct {
|
||||
old models.ChatMemberType
|
||||
canSend bool
|
||||
}{
|
||||
{models.ChatMemberTypeLeft, false},
|
||||
{models.ChatMemberTypeRestricted, false},
|
||||
{models.ChatMemberTypeRestricted, true},
|
||||
} {
|
||||
t.Run(fmt.Sprintf("%s_cansend=%v", tc.old, tc.canSend), func(t *testing.T) {
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, tc.old, tc.canSend))
|
||||
if len(api.restricts) != 1 || !api.restricts[0].canSend {
|
||||
t.Fatalf("restricts = %+v, want one grant for a restricted member", api.restricts)
|
||||
}
|
||||
})
|
||||
func TestHandleChatMemberLeavesEligibleMemberAlone(t *testing.T) {
|
||||
// An eligible plain member already sends (the permissive default); no action needed.
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
b.handleChatMember(context.Background(), memberUpdate(testChatID, 777, models.ChatMemberTypeLeft))
|
||||
if len(api.restricts) != 0 {
|
||||
t.Fatalf("restricts = %+v, want none for an eligible member (already allowed)", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberUnmutesEligibleRestricted(t *testing.T) {
|
||||
// An eligible member the bot had muted (restricted, can_send=false) is restored.
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeRestricted, false, true))
|
||||
if len(api.restricts) != 1 || !api.restricts[0].canSend {
|
||||
t.Fatalf("restricts = %+v, want one un-mute (can_send=true)", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberLeavesEligibleAllowedRestrictedAlone(t *testing.T) {
|
||||
// An eligible restricted member who can already send needs no change — the real case
|
||||
// from the contour (restricted, can_send=true, eligible).
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeRestricted, true, true))
|
||||
if len(api.restricts) != 0 {
|
||||
t.Fatalf("restricts = %+v, want none for an eligible already-allowed member", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberMutesIneligibleRestricted(t *testing.T) {
|
||||
// An ineligible member who can still send is muted.
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, false, nil)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeRestricted, true, true))
|
||||
if len(api.restricts) != 1 || api.restricts[0].canSend {
|
||||
t.Fatalf("restricts = %+v, want one mute (can_send=false)", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberSkipsNonMember(t *testing.T) {
|
||||
// A restricted record for a user no longer in the chat (is_member=false) is not acted on.
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, false, nil)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeRestricted, true, false))
|
||||
if len(api.restricts) != 0 {
|
||||
t.Fatalf("restricts = %+v, want none for a non-member", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberSkipsBotsOwnAction(t *testing.T) {
|
||||
// The bot's own grant re-fires a chat_member update performed by the bot; it must be
|
||||
// skipped so a grant never loops into another grant.
|
||||
// The bot's own restrict re-fires a chat_member update performed by the bot; skip it
|
||||
// so an action never loops (the resolver here would otherwise mute).
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
upd := restrictedUpdate(testChatID, 777, models.ChatMemberTypeRestricted, false)
|
||||
b := eligibleBot(t, api, false, nil)
|
||||
upd := memberUpdate(testChatID, 777, models.ChatMemberTypeLeft)
|
||||
upd.From = models.User{ID: botSelfID}
|
||||
b.handleChatMember(context.Background(), upd)
|
||||
if len(api.restricts) != 0 {
|
||||
@@ -150,30 +177,21 @@ func TestHandleChatMemberSkipsBotsOwnAction(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberSkipsIneligible(t *testing.T) {
|
||||
func TestHandleChatMemberNoChangeOnResolveError(t *testing.T) {
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, false, nil)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeLeft, false))
|
||||
b := eligibleBot(t, api, false, context.DeadlineExceeded)
|
||||
b.handleChatMember(context.Background(), memberUpdate(testChatID, 777, models.ChatMemberTypeLeft))
|
||||
if len(api.restricts) != 0 {
|
||||
t.Fatalf("an ineligible member was granted: %+v (want left muted)", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberFailsClosedOnResolveError(t *testing.T) {
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, context.DeadlineExceeded)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeLeft, false))
|
||||
if len(api.restricts) != 0 {
|
||||
t.Fatalf("a resolve error still granted write: %+v (want fail-closed)", api.restricts)
|
||||
t.Fatalf("a resolve error still changed access: %+v (want no change)", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleChatMemberIgnoresOtherChatAndLeaves(t *testing.T) {
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
b := eligibleBot(t, api, false, nil) // ineligible — would mute if it acted
|
||||
ctx := context.Background()
|
||||
b.handleChatMember(ctx, restrictedUpdate(999, 777, models.ChatMemberTypeLeft, false)) // foreign chat
|
||||
b.handleChatMember(ctx, leftUpdate(testChatID, 777)) // a leave
|
||||
b.handleChatMember(ctx, memberUpdate(999, 777, models.ChatMemberTypeLeft)) // foreign chat
|
||||
b.handleChatMember(ctx, leftUpdate(testChatID, 777)) // a leave
|
||||
if len(api.restricts) != 0 {
|
||||
t.Fatalf("restricts = %+v, want none for a foreign chat / a leave", api.restricts)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user