From a4045130376873fc47fbd83ee83939ddb87b848b Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Sun, 21 Jun 2026 15:19:21 +0200 Subject: [PATCH] feat(telegram): chat-gate observability + grant on first registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from a contour test where a user joined the chat, then registered, and got no write access — with silent logs. Observability: log every chat_member update (chat id, configured id, user, old->new status), the eligibility result and the grant outcome; plus a startup self-check that warns loudly when the bot is not an administrator in the chat with the restrict-members ("Ban users") right — the common misconfiguration, previously invisible in the logs. Grant on first registration: a user who joins the moderated chat BEFORE registering is covered by no chat_member event, so the join-time grant never fires for them. ProvisionTelegram now reports first contact, and the Telegram auth handler emits chat_access_changed on it, so the gateway re-evaluates and grants write access if the user is already in the chat. --- backend/internal/account/account.go | 28 +++++++--- backend/internal/inttest/account_test.go | 18 ++++--- backend/internal/inttest/admin_test.go | 2 +- backend/internal/inttest/chat_access_test.go | 45 +++++++++++++++- .../internal/inttest/suspension_gate_test.go | 2 +- backend/internal/inttest/userlist_test.go | 2 +- backend/internal/server/handlers_auth.go | 8 ++- platform/telegram/internal/bot/bot.go | 52 ++++++++++++++++++- 8 files changed, 136 insertions(+), 21 deletions(-) diff --git a/backend/internal/account/account.go b/backend/internal/account/account.go index 1cdacbc..816ba4b 100644 --- a/backend/internal/account/account.go +++ b/backend/internal/account/account.go @@ -151,14 +151,26 @@ func (s *Store) ProvisionRobot(ctx context.Context, externalID, displayName stri return modelToAccount(row), nil } -// ProvisionTelegram provisions (or finds) the account bound to a Telegram -// identity. On first contact only, it seeds the new account's preferred language -// from the Telegram client languageCode (when it maps to a supported language) and -// its display name sanitized from firstName (falling back to username, then to a -// generated placeholder when neither yields any letters); an already-existing -// account is returned unchanged, so a later profile edit is never overwritten. -func (s *Store) ProvisionTelegram(ctx context.Context, externalID, languageCode, username, firstName string) (Account, error) { - return s.provision(ctx, KindTelegram, externalID, telegramSeed(languageCode, username, firstName)) +// ProvisionTelegram provisions (or finds) the account bound to a Telegram identity, +// reporting whether this call created it (first contact). On first contact only, it +// seeds the new account's preferred language from the Telegram client languageCode +// (when it maps to a supported language) and its display name sanitized from firstName +// (falling back to username, then to a generated placeholder when neither yields any +// letters); an already-existing account is returned unchanged, so a later profile edit +// is never overwritten. The created flag lets the auth handler re-evaluate moderated- +// chat write access on first registration — the path of a user who joined the chat +// before registering, whom no chat_member event covers. +func (s *Store) ProvisionTelegram(ctx context.Context, externalID, languageCode, username, firstName string) (Account, bool, error) { + // Pre-check whether the identity already exists so the caller can act on first + // contact. A race with a concurrent create only over- or under-reports created for + // that one call, which the idempotent chat-access re-evaluation tolerates. + _, err := s.findByIdentity(ctx, KindTelegram, externalID) + created := errors.Is(err, ErrNotFound) + if err != nil && !created { + return Account{}, false, err + } + acc, err := s.provision(ctx, KindTelegram, externalID, telegramSeed(languageCode, username, firstName)) + return acc, created, err } // provision finds the account for (kind, externalID) or creates it with seed, diff --git a/backend/internal/inttest/account_test.go b/backend/internal/inttest/account_test.go index b9327a7..67e002c 100644 --- a/backend/internal/inttest/account_test.go +++ b/backend/internal/inttest/account_test.go @@ -118,10 +118,13 @@ func TestProvisionTelegramSeedsNewAccountOnly(t *testing.T) { store := account.NewStore(testDB) ext := "tg-" + uuid.NewString() - acc, err := store.ProvisionTelegram(ctx, ext, "ru-RU", "thehandle", "Иван") + acc, created, err := store.ProvisionTelegram(ctx, ext, "ru-RU", "thehandle", "Иван") if err != nil { t.Fatalf("provision telegram: %v", err) } + if !created { + t.Error("created = false on first contact, want true") + } if acc.PreferredLanguage != "ru" { t.Errorf("PreferredLanguage = %q, want ru", acc.PreferredLanguage) } @@ -133,10 +136,13 @@ func TestProvisionTelegramSeedsNewAccountOnly(t *testing.T) { } // A later login with different fields returns the same account, unchanged. - again, err := store.ProvisionTelegram(ctx, ext, "en", "other", "Other") + again, created, err := store.ProvisionTelegram(ctx, ext, "en", "other", "Other") if err != nil { t.Fatalf("re-provision telegram: %v", err) } + if created { + t.Error("created = true on a repeat login, want false") + } if again.ID != acc.ID { t.Errorf("re-provision id = %s, want %s", again.ID, acc.ID) } @@ -150,7 +156,7 @@ func TestProvisionTelegramSeedsNewAccountOnly(t *testing.T) { // language CHECK. func TestProvisionTelegramUnknownLanguageDefaults(t *testing.T) { ctx := context.Background() - acc, err := account.NewStore(testDB).ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "fr", "", "") + acc, _, err := account.NewStore(testDB).ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "fr", "", "") if err != nil { t.Fatalf("provision telegram: %v", err) } @@ -166,7 +172,7 @@ func TestProvisionTelegramUnknownLanguageDefaults(t *testing.T) { func TestHighRateFlagRoundTrip(t *testing.T) { ctx := context.Background() store := account.NewStore(testDB) - acc, err := store.ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "en", "", "Player") + acc, _, err := store.ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "en", "", "Player") if err != nil { t.Fatalf("provision telegram: %v", err) } @@ -222,7 +228,7 @@ func TestIdentityExternalID(t *testing.T) { ctx := context.Background() store := account.NewStore(testDB) ext := "tg-" + uuid.NewString() - acc, err := store.ProvisionTelegram(ctx, ext, "en", "", "Tg User") + acc, _, err := store.ProvisionTelegram(ctx, ext, "en", "", "Tg User") if err != nil { t.Fatalf("provision telegram: %v", err) } @@ -247,7 +253,7 @@ func TestIdentityExternalID(t *testing.T) { func TestNotificationsInAppOnlyRoundTrip(t *testing.T) { ctx := context.Background() store := account.NewStore(testDB) - acc, err := store.ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "en", "", "Player") + acc, _, err := store.ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "en", "", "Player") if err != nil { t.Fatalf("provision telegram: %v", err) } diff --git a/backend/internal/inttest/admin_test.go b/backend/internal/inttest/admin_test.go index 86b1c46..46f3689 100644 --- a/backend/internal/inttest/admin_test.go +++ b/backend/internal/inttest/admin_test.go @@ -222,7 +222,7 @@ func TestConsoleGameDetailRobotSchedule(t *testing.T) { func TestConsoleThrottledViewAndFlagClear(t *testing.T) { ctx := context.Background() accounts := account.NewStore(testDB) - acc, err := accounts.ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "en", "", "Throttled Player") + acc, _, err := accounts.ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "en", "", "Throttled Player") if err != nil { t.Fatalf("provision: %v", err) } diff --git a/backend/internal/inttest/chat_access_test.go b/backend/internal/inttest/chat_access_test.go index 065af0d..b36599b 100644 --- a/backend/internal/inttest/chat_access_test.go +++ b/backend/internal/inttest/chat_access_test.go @@ -18,6 +18,7 @@ import ( "scrabble/backend/internal/account" "scrabble/backend/internal/notify" "scrabble/backend/internal/server" + "scrabble/backend/internal/session" ) // chatAccessBody mirrors the backend's /internal/chat-access JSON for the test. @@ -54,7 +55,7 @@ func TestChatAccessResolver(t *testing.T) { srv := server.New(":0", server.Deps{Logger: zaptest.NewLogger(t), DB: testDB, Accounts: accounts}) ext := "tg-" + uuid.NewString() - acc, err := accounts.ProvisionTelegram(ctx, ext, "en", "", "Chatter") + acc, _, err := accounts.ProvisionTelegram(ctx, ext, "en", "", "Chatter") if err != nil { t.Fatalf("provision: %v", err) } @@ -203,6 +204,48 @@ func TestChatAccessPublishedOnModeration(t *testing.T) { } } +// TestChatAccessPublishedOnFirstRegistration checks that a Telegram first contact +// (the sessions/telegram endpoint creating the account) emits chat_access_changed — +// the re-grant for a user who joined the moderated chat before registering — and that +// a repeat login does not re-emit. +func TestChatAccessPublishedOnFirstRegistration(t *testing.T) { + notifier := &captureNotifier{} + srv := server.New(":0", server.Deps{ + Logger: zaptest.NewLogger(t), + DB: testDB, + Accounts: account.NewStore(testDB), + Sessions: session.NewService(session.NewStore(testDB), session.NewCache()), + Notifier: notifier, + }) + h := srv.Handler() + ext := "tg-" + uuid.NewString() + + post := func() { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/v1/internal/sessions/telegram", + strings.NewReader(`{"external_id":"`+ext+`","language_code":"en","first_name":"Reg"}`)) + req.Header.Set("Content-Type", "application/json") + h.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("telegram auth = %d: %s", rec.Code, rec.Body.String()) + } + } + + post() + acc, err := account.NewStore(testDB).AccountByIdentity(context.Background(), account.KindTelegram, ext) + if err != nil { + t.Fatalf("lookup: %v", err) + } + if got := notifier.count(acc.ID, notify.KindChatAccessChanged); got != 1 { + t.Fatalf("first registration: chat_access_changed count = %d, want 1", got) + } + // A repeat login (the account already exists) must not re-emit. + post() + if got := notifier.count(acc.ID, notify.KindChatAccessChanged); got != 1 { + t.Fatalf("repeat login: chat_access_changed count = %d, want still 1", got) + } +} + // TestSuspensionsExpiredBetween checks the sweeper's window query: a non-lifted // temporary block whose expiry falls in the window is returned, while one outside the // window, a permanent block, and a lifted block are not. diff --git a/backend/internal/inttest/suspension_gate_test.go b/backend/internal/inttest/suspension_gate_test.go index 27555f8..3d123af 100644 --- a/backend/internal/inttest/suspension_gate_test.go +++ b/backend/internal/inttest/suspension_gate_test.go @@ -38,7 +38,7 @@ func TestSuspensionGate(t *testing.T) { Accounts: accounts, }) - acc, err := accounts.ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "ru", "", "Blocked") + acc, _, err := accounts.ProvisionTelegram(ctx, "tg-"+uuid.NewString(), "ru", "", "Blocked") if err != nil { t.Fatalf("provision: %v", err) } diff --git a/backend/internal/inttest/userlist_test.go b/backend/internal/inttest/userlist_test.go index 8e47e5f..78a4e2c 100644 --- a/backend/internal/inttest/userlist_test.go +++ b/backend/internal/inttest/userlist_test.go @@ -18,7 +18,7 @@ func TestUserListFilter(t *testing.T) { st := account.NewStore(testDB) uniq := uuid.NewString() - human, err := st.ProvisionTelegram(ctx, "tg-"+uniq, "en", "", "Zzqxhuman") + human, _, err := st.ProvisionTelegram(ctx, "tg-"+uniq, "en", "", "Zzqxhuman") if err != nil { t.Fatalf("provision human: %v", err) } diff --git a/backend/internal/server/handlers_auth.go b/backend/internal/server/handlers_auth.go index 59bc100..06cdf72 100644 --- a/backend/internal/server/handlers_auth.go +++ b/backend/internal/server/handlers_auth.go @@ -35,11 +35,17 @@ func (s *Server) handleTelegramAuth(c *gin.Context) { abortBadRequest(c, "external_id is required") return } - acc, err := s.accounts.ProvisionTelegram(c.Request.Context(), req.ExternalID, req.LanguageCode, req.Username, req.FirstName) + acc, created, err := s.accounts.ProvisionTelegram(c.Request.Context(), req.ExternalID, req.LanguageCode, req.Username, req.FirstName) if err != nil { s.abortErr(c, err) return } + if created { + // First registration: re-evaluate moderated-chat write access, so a user who + // joined the chat before registering is granted on the spot (no chat_member + // event fires on registration). + s.publishChatAccessChange(acc.ID) + } s.mintSession(c, acc) } diff --git a/platform/telegram/internal/bot/bot.go b/platform/telegram/internal/bot/bot.go index c88fd06..6cb3b45 100644 --- a/platform/telegram/internal/bot/bot.go +++ b/platform/telegram/internal/bot/bot.go @@ -117,9 +117,38 @@ func (t *Bot) Run(ctx context.Context) { }); err != nil { t.log.Warn("set menu button failed", zap.Error(err)) } + if t.chatID != 0 { + t.logChatAdminStatus(ctx) + } t.api.Start(ctx) } +// logChatAdminStatus checks, at startup, whether the bot can actually gate the +// moderated chat — it must be an administrator there with the restrict-members +// ("Ban users") right, or Telegram delivers no chat_member updates and restricts +// fail. It logs a prominent warning when the prerequisite is missing (the common +// misconfiguration), so the cause is visible without reproducing a join. +func (t *Bot) logChatAdminStatus(ctx context.Context) { + me, err := t.api.GetMe(ctx) + if err != nil { + t.log.Warn("chat self-check: getMe failed", zap.Error(err)) + return + } + m, err := t.api.GetChatMember(ctx, &tgbot.GetChatMemberParams{ChatID: t.chatID, UserID: me.ID}) + if err != nil { + t.log.Warn("chat gating self-check failed: the bot cannot read the chat — is it added and is TELEGRAM_CHAT_ID the discussion group id?", + zap.Int64("chat_id", t.chatID), zap.Error(err)) + return + } + canRestrict := m.Type == models.ChatMemberTypeAdministrator && m.Administrator.CanRestrictMembers + if !canRestrict { + t.log.Warn(`chat gating WILL NOT WORK: the bot must be an administrator with the restrict-members ("Ban users") right`, + zap.Int64("chat_id", t.chatID), zap.String("bot_status", string(m.Type))) + return + } + t.log.Info("chat gating ready: bot is an admin with the restrict-members right", zap.Int64("chat_id", t.chatID)) +} + // Notify sends a notification message with a Mini App launch button that opens the // app at startParam (empty opens the lobby). func (t *Bot) Notify(ctx context.Context, chatID int64, text, buttonText, startParam string) error { @@ -226,6 +255,20 @@ func (t *Bot) SetEligibilityResolver(resolve EligibilityResolver) { // defaults to no-send), and a resolve failure fails closed (also left muted). Other // status changes (leaves, restrictions, admin edits) are ignored. func (t *Bot) handleChatMember(ctx context.Context, cm *models.ChatMemberUpdated) { + user := chatMemberUser(cm.NewChatMember) + var uid int64 + if user != nil { + uid = user.ID + } + // Log every chat_member update the bot receives: this is the one place to see + // whether Telegram is delivering joins at all, for which chat, and the transition. + t.log.Info("chat_member update", + zap.Int64("chat_id", cm.Chat.ID), + zap.Int64("configured_chat_id", t.chatID), + zap.Int64("user_id", uid), + zap.String("old_status", string(cm.OldChatMember.Type)), + zap.String("new_status", string(cm.NewChatMember.Type))) + if t.chatID == 0 || cm.Chat.ID != t.chatID { return } @@ -233,24 +276,27 @@ func (t *Bot) handleChatMember(ctx context.Context, cm *models.ChatMemberUpdated if cm.NewChatMember.Type != models.ChatMemberTypeMember || cm.OldChatMember.Type == models.ChatMemberTypeMember { return } - user := chatMemberUser(cm.NewChatMember) if user == nil || user.IsBot { return } if t.eligibility == nil { - return // resolver not wired: leave muted (fail closed) + t.log.Warn("chat join: eligibility resolver not wired", zap.Int64("user_id", uid)) + return } eligible, err := t.eligibility(ctx, strconv.FormatInt(user.ID, 10)) if err != nil { t.log.Warn("chat join eligibility failed", zap.Int64("user_id", user.ID), zap.Error(err)) return } + t.log.Info("chat join evaluated", zap.Int64("user_id", user.ID), zap.Bool("eligible", eligible)) if !eligible { return // not registered or blocked: leave muted } if err := t.setChatWrite(ctx, user.ID, true); err != nil { t.log.Warn("grant chat write failed", zap.Int64("user_id", user.ID), zap.Error(err)) + return } + t.log.Info("chat write granted", zap.Int64("user_id", user.ID)) } // ApplyChatGate applies a chat-gate command (an admin block/unblock or chat_muted @@ -272,8 +318,10 @@ func (t *Bot) ApplyChatGate(ctx context.Context, userID int64, allow bool) (bool if err := t.setChatWrite(ctx, userID, allow); err != nil { return false, err } + t.log.Info("chat gate applied", zap.Int64("user_id", userID), zap.Bool("allow", allow)) return true, nil default: + t.log.Info("chat gate: user not in chat, skipped", zap.Int64("user_id", userID), zap.String("status", string(member.Type))) return false, nil // absent, or an admin/owner who cannot be restricted } }