From 42033f170e851cf297672d7708df04a2085ce9bd Mon Sep 17 00:00:00 2001 From: Umputun Date: Sat, 14 Dec 2024 12:49:14 -0600 Subject: [PATCH 1/2] make clean message extraction optional and change error to the logged report --- app/events/admin.go | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/app/events/admin.go b/app/events/admin.go index 06a4a09..c80bdc8 100644 --- a/app/events/admin.go +++ b/app/events/admin.go @@ -412,13 +412,15 @@ func (a *admin) callbackBanConfirmed(query *tbapi.CallbackQuery) error { return fmt.Errorf("failed to clear confirmation, chatID:%d, msgID:%d, %w", query.Message.Chat.ID, query.Message.MessageID, err) } - cleanMsg, err := a.getCleanMessage(query.Message.Text) - if err != nil { - return fmt.Errorf("failed to get clean message: %w", err) - } - - if err = a.bot.UpdateSpam(cleanMsg); err != nil { // update spam samples - return fmt.Errorf("failed to update spam for %q: %w", cleanMsg, err) + if cleanMsg, err := a.getCleanMessage(query.Message.Text); err != nil { + // we don't want to fail on this error, as lack of a clean message should not prevent deleteAndBan + // for soft and training modes, we just don't need to update spam samples with empty messages. + log.Printf("[DEBUG] failed to get clean message: %v", err) + } else { + // update spam samples if we have a clean message + if err = a.bot.UpdateSpam(cleanMsg); err != nil { // update spam samples + return fmt.Errorf("failed to update spam for %q: %w", cleanMsg, err) + } } userID, msgID, parseErr := a.parseCallbackData(query.Data) @@ -428,12 +430,12 @@ func (a *admin) callbackBanConfirmed(query *tbapi.CallbackQuery) error { if a.trainingMode { // in training mode, the user is not banned automatically, here we do the real ban & delete the message - if err = a.deleteAndBan(query, userID, msgID); err != nil { + if err := a.deleteAndBan(query, userID, msgID); err != nil { return fmt.Errorf("failed to ban user %d: %w", userID, err) } } - // for soft ban we need to ba user for real on confirmation + // for soft ban we need to ban user for real on confirmation if a.softBan && !a.trainingMode { userName, err := a.extractUsername(query.Message.Text) // try to extract username from the message if err != nil { @@ -470,12 +472,14 @@ func (a *admin) callbackUnbanConfirmed(query *tbapi.CallbackQuery) error { } // get the original spam message to update ham samples - cleanMsg, err := a.getCleanMessage(query.Message.Text) - if err != nil { - return fmt.Errorf("failed to get clean message: %w", err) - } - if derr := a.bot.UpdateHam(cleanMsg); derr != nil { - return fmt.Errorf("failed to update ham for %q: %w", cleanMsg, derr) + if cleanMsg, cleanErr := a.getCleanMessage(query.Message.Text); cleanErr != nil { + // we don't want to fail on this error, as lack of a clean message should not prevent unban action + log.Printf("[DEBUG] failed to get clean message: %v", cleanErr) + } else { + // update ham samples if we have a clean message + if upErr := a.bot.UpdateHam(cleanMsg); upErr != nil { + return fmt.Errorf("failed to update ham for %q: %w", cleanMsg, upErr) + } } // unban user if not in training mode (in training mode, the user is not banned automatically) From 06b1dc6a12ee75b91fecb9f8f4a8b26825e81623 Mon Sep 17 00:00:00 2001 From: Umputun Date: Sat, 14 Dec 2024 13:12:17 -0600 Subject: [PATCH 2/2] add a test and extra check --- app/events/admin.go | 19 +++++----- app/events/listener_test.go | 75 +++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/app/events/admin.go b/app/events/admin.go index c80bdc8..359e9f0 100644 --- a/app/events/admin.go +++ b/app/events/admin.go @@ -412,15 +412,14 @@ func (a *admin) callbackBanConfirmed(query *tbapi.CallbackQuery) error { return fmt.Errorf("failed to clear confirmation, chatID:%d, msgID:%d, %w", query.Message.Chat.ID, query.Message.MessageID, err) } - if cleanMsg, err := a.getCleanMessage(query.Message.Text); err != nil { - // we don't want to fail on this error, as lack of a clean message should not prevent deleteAndBan - // for soft and training modes, we just don't need to update spam samples with empty messages. - log.Printf("[DEBUG] failed to get clean message: %v", err) - } else { - // update spam samples if we have a clean message + if cleanMsg, err := a.getCleanMessage(query.Message.Text); err == nil && cleanMsg != "" { if err = a.bot.UpdateSpam(cleanMsg); err != nil { // update spam samples return fmt.Errorf("failed to update spam for %q: %w", cleanMsg, err) } + } else { + // we don't want to fail on this error, as lack of a clean message should not prevent deleteAndBan + // for soft and training modes, we just don't need to update spam samples with empty messages. + log.Printf("[DEBUG] failed to get clean message: %v", err) } userID, msgID, parseErr := a.parseCallbackData(query.Data) @@ -472,14 +471,14 @@ func (a *admin) callbackUnbanConfirmed(query *tbapi.CallbackQuery) error { } // get the original spam message to update ham samples - if cleanMsg, cleanErr := a.getCleanMessage(query.Message.Text); cleanErr != nil { - // we don't want to fail on this error, as lack of a clean message should not prevent unban action - log.Printf("[DEBUG] failed to get clean message: %v", cleanErr) - } else { + if cleanMsg, cleanErr := a.getCleanMessage(query.Message.Text); cleanErr == nil && cleanMsg != "" { // update ham samples if we have a clean message if upErr := a.bot.UpdateHam(cleanMsg); upErr != nil { return fmt.Errorf("failed to update ham for %q: %w", cleanMsg, upErr) } + } else { + // we don't want to fail on this error, as lack of a clean message should not prevent unban action + log.Printf("[DEBUG] failed to get clean message: %v", cleanErr) } // unban user if not in training mode (in training mode, the user is not banned automatically) diff --git a/app/events/listener_test.go b/app/events/listener_test.go index f8afe67..8270186 100644 --- a/app/events/listener_test.go +++ b/app/events/listener_test.go @@ -893,6 +893,81 @@ func TestTelegramListener_DoWithAdminSoftUnBan(t *testing.T) { assert.Equal(t, int64(777), b.AddApprovedUserCalls()[0].ID) } +func TestTelegramListener_DoWithAdminSoftUnBanEmptyText(t *testing.T) { + mockLogger := &mocks.SpamLoggerMock{} + mockAPI := &mocks.TbAPIMock{ + GetChatFunc: func(config tbapi.ChatInfoConfig) (tbapi.ChatFullInfo, error) { + return tbapi.ChatFullInfo{Chat: tbapi.Chat{ID: 123}}, nil + }, + SendFunc: func(c tbapi.Chattable) (tbapi.Message, error) { + if mc, ok := c.(tbapi.MessageConfig); ok { + return tbapi.Message{Text: mc.Text, From: &tbapi.User{UserName: "user"}}, nil + } + return tbapi.Message{}, nil + }, + RequestFunc: func(c tbapi.Chattable) (*tbapi.APIResponse, error) { + return &tbapi.APIResponse{}, nil + }, + GetChatAdministratorsFunc: func(config tbapi.ChatAdministratorsConfig) ([]tbapi.ChatMember, error) { return nil, nil }, + } + b := &mocks.BotMock{ + UpdateHamFunc: func(msg string) error { + return nil + }, + AddApprovedUserFunc: func(id int64, name string) error { return nil }, + } + + locator, teardown := prepTestLocator(t) + defer teardown() + + l := TelegramListener{ + SpamLogger: mockLogger, + TbAPI: mockAPI, + Bot: b, + SuperUsers: SuperUsers{"admin"}, + Group: "gr", + Locator: locator, + AdminGroup: "123", + SoftBanMode: true, + } + + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Minute) + defer cancel() + + updMsg := tbapi.Update{ + CallbackQuery: &tbapi.CallbackQuery{ + Data: "777:999", + Message: &tbapi.Message{ + MessageID: 987654, + Chat: tbapi.Chat{ID: 123}, + Text: "unban user blah\n\n", + From: &tbapi.User{UserName: "user", ID: 999}, + ForwardOrigin: &tbapi.MessageOrigin{Date: time.Date(2020, 2, 11, 19, 35, 55, 9, time.UTC).Unix()}, + }, + From: &tbapi.User{UserName: "admin", ID: 1000}, + }, + } + updChan := make(chan tbapi.Update, 1) + updChan <- updMsg + close(updChan) + mockAPI.GetUpdatesChanFunc = func(config tbapi.UpdateConfig) tbapi.UpdatesChannel { return updChan } + + err := l.Do(ctx) + assert.EqualError(t, err, "telegram update chan closed") + require.Equal(t, 1, len(mockAPI.SendCalls())) + assert.Equal(t, 987654, mockAPI.SendCalls()[0].C.(tbapi.EditMessageTextConfig).MessageID) + assert.Contains(t, mockAPI.SendCalls()[0].C.(tbapi.EditMessageTextConfig).Text, "by admin in ") + require.Equal(t, 2, len(mockAPI.RequestCalls())) + assert.Equal(t, "accepted", mockAPI.RequestCalls()[0].C.(tbapi.CallbackConfig).Text) + + assert.Equal(t, int64(777), mockAPI.RequestCalls()[1].C.(tbapi.RestrictChatMemberConfig).UserID) + assert.Equal(t, &tbapi.ChatPermissions{CanSendMessages: true, CanSendAudios: true, CanSendDocuments: true, CanSendPhotos: true, CanSendVideos: true, CanSendVideoNotes: true, CanSendVoiceNotes: true, CanSendOtherMessages: true, CanChangeInfo: true, CanInviteUsers: true, CanPinMessages: true}, + mockAPI.RequestCalls()[1].C.(tbapi.RestrictChatMemberConfig).Permissions) + require.Equal(t, 0, len(b.UpdateHamCalls())) + require.Equal(t, 1, len(b.AddApprovedUserCalls())) + assert.Equal(t, int64(777), b.AddApprovedUserCalls()[0].ID) +} + func TestTelegramListener_DoWithAdminUnBan_Training(t *testing.T) { mockLogger := &mocks.SpamLoggerMock{} mockAPI := &mocks.TbAPIMock{