fix(tgbot): scope the add-client wizard to the admin, not the chat (#6604)

Two admins in one group chat shared one draft and one wizard step: clientDrafts and userStateStore were keyed by chat id alone, so the second admin's wizard opened on the first one's email and limits, and whichever of them tapped a control last decided what the other created.

Key both stores by (chat, user) instead. A private chat is unaffected: its two ids are equal, so the key matches what the chat alone used to be. A message with no sender (a channel post) keys to user 0, which no admin holds.

Fixes #6593.

internal/web/service/tgbot/tgbot.go

@@ -82,30 +82,52 @@ type clientDraft struct {
 	reset              int
 }
 
-// clientDrafts keys a draft by chat: the steps arrive on the worker pool, so a
-// single draft let two admins fill in one client between them.
+// chatUser names the admin a wizard belongs to. A private chat's ids are equal;
+// in a group they are not, and each admin at its keyboard fills in their own.
+type chatUser struct {
+	chatID int64
+	userID int64
+}
+
+// messageActor reads the sender off a message. A post without one (a channel)
+// keys to user 0, an id no admin can hold.
+func messageActor(message telego.Message) chatUser {
+	if message.From == nil {
+		return chatUser{chatID: message.Chat.ID}
+	}
+	return chatUser{chatID: message.Chat.ID, userID: message.From.ID}
+}
+
+// callbackActor reads the admin who tapped the button, not the chat the keyboard
+// sits in: every admin in a group sees the same keyboard.
+func callbackActor(callbackQuery *telego.CallbackQuery) chatUser {
+	return chatUser{chatID: callbackQuery.Message.GetChat().ID, userID: callbackQuery.From.ID}
+}
+
+// clientDrafts keys a draft by the admin filling it in: the steps arrive on the
+// worker pool, so one draft let two admins fill in one client between them.
 type clientDrafts struct {
 	mu     sync.Mutex
-	drafts map[int64]*clientDraft
+	drafts map[chatUser]*clientDraft
 }
 
-var addClientDrafts = &clientDrafts{drafts: make(map[int64]*clientDraft)}
+var addClientDrafts = &clientDrafts{drafts: make(map[chatUser]*clientDraft)}
 
-func (s *clientDrafts) forChat(chatID int64) *clientDraft {
+func (s *clientDrafts) forActor(actor chatUser) *clientDraft {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	draft, ok := s.drafts[chatID]
+	draft, ok := s.drafts[actor]
 	if !ok {
 		draft = &clientDraft{}
-		s.drafts[chatID] = draft
+		s.drafts[actor] = draft
 	}
 	return draft
 }
 
-func (s *clientDrafts) reset(chatID int64) {
+func (s *clientDrafts) reset(actor chatUser) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	delete(s.drafts, chatID)
+	delete(s.drafts, actor)
 }
 
 // isAddClientStep reports whether callback data belongs to the add-client
@@ -117,17 +139,17 @@ func isAddClientStep(data string) bool {
 func (s *clientDrafts) resetAll() {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	s.drafts = make(map[int64]*clientDraft)
+	s.drafts = make(map[chatUser]*clientDraft)
 }
 
-// userStateStore guards the per-chat conversation states. The Telegram command
+// userStateStore guards the per-admin conversation states. The Telegram command
 // and callback handlers run on a worker-pool goroutine while the message handler
 // runs on the dispatch goroutine, so a bare map would be a concurrent-map-write
 // crash. It also expires abandoned conversations so a user who starts a flow and
 // goes silent doesn't leave an entry forever.
 type userStateStore struct {
 	mu        sync.Mutex
-	states    map[int64]userStateEntry
+	states    map[chatUser]userStateEntry
 	lastPrune time.Time
 }
 
@@ -136,30 +158,30 @@ type userStateEntry struct {
 	at    time.Time
 }
 
-var userStateMgr = &userStateStore{states: make(map[int64]userStateEntry)}
+var userStateMgr = &userStateStore{states: make(map[chatUser]userStateEntry)}
 
-func (s *userStateStore) set(chatID int64, state string) {
+func (s *userStateStore) set(actor chatUser, state string) {
 	s.mu.Lock()
-	s.states[chatID] = userStateEntry{state: state, at: time.Now()}
+	s.states[actor] = userStateEntry{state: state, at: time.Now()}
 	s.mu.Unlock()
 }
 
-func (s *userStateStore) get(chatID int64) (string, bool) {
+func (s *userStateStore) get(actor chatUser) (string, bool) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	e, ok := s.states[chatID]
+	e, ok := s.states[actor]
 	return e.state, ok
 }
 
-func (s *userStateStore) clear(chatID int64) {
+func (s *userStateStore) clear(actor chatUser) {
 	s.mu.Lock()
-	delete(s.states, chatID)
+	delete(s.states, actor)
 	s.mu.Unlock()
 }
 
 func (s *userStateStore) reset() {
 	s.mu.Lock()
-	s.states = make(map[int64]userStateEntry)
+	s.states = make(map[chatUser]userStateEntry)
 	s.mu.Unlock()
 }
 

internal/web/service/tgbot/tgbot_add_client_expiry_test.go

@@ -20,10 +20,10 @@ func TestAddClientExpiryPresetReplacesTheTerm(t *testing.T) {
 	swapTestBot(t, url)
 
 	// A fresh draft attaches no inbound, so the card never looks one up by remark.
-	draft := addClientDrafts.forChat(chatID)
+	draft := addClientDrafts.forActor(chatUser{chatID: chatID, userID: 1})
 	origRunning := isRunning
 	t.Cleanup(func() {
-		addClientDrafts.reset(chatID)
+		addClientDrafts.reset(chatUser{chatID: chatID, userID: 1})
 		isRunning = origRunning
 	})
 	isRunning = true

internal/web/service/tgbot/tgbot_client_draft_per_admin_test.go

@@ -0,0 +1,102 @@
+package tgbot
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
+
+	"github.com/mymmrac/telego"
+)
+
+// Regression test: keying the add-client wizard by chat alone left the two
+// admins of a group chat filling in one client between them.
+func TestAddClientDraftIsPerAdminInGroupChat(t *testing.T) {
+	if err := database.InitDB(filepath.Join(t.TempDir(), "x-ui.db")); err != nil {
+		t.Fatalf("InitDB: %v", err)
+	}
+	t.Cleanup(func() { _ = database.CloseDB() })
+
+	const (
+		groupChat = int64(-1001234567890)
+		adminA    = int64(8101)
+		adminB    = int64(8202)
+	)
+	url, textsFor := draftTexts(t)
+	swapTestBot(t, url)
+	origRunning := isRunning
+	t.Cleanup(func() {
+		isRunning = origRunning
+		userStateMgr.reset()
+	})
+	isRunning = true
+
+	// Both admins tap in the same chat; only the sender tells them apart.
+	tap := func(userID int64, data string) {
+		t.Helper()
+		(&Tgbot{}).answerCallback(&telego.CallbackQuery{
+			ID:      "q1",
+			From:    telego.User{ID: userID},
+			Data:    data,
+			Message: &telego.Message{MessageID: 7, Chat: telego.Chat{ID: groupChat}},
+		}, true)
+	}
+
+	tap(adminA, "add_client_to 1")
+	emailA := cardEmail(t, lastDraftCard(t, textsFor(groupChat)))
+	tap(adminB, "add_client_to 2")
+	emailB := cardEmail(t, lastDraftCard(t, textsFor(groupChat)))
+	if emailA == "" || emailA == emailB {
+		t.Fatalf("both admins start with email %q, want one draft per admin", emailA)
+	}
+
+	// Admin A renders again, with admin B's wizard already past its start.
+	tap(adminA, "add_client_default_traffic_exp")
+	if got := cardEmail(t, lastDraftCard(t, textsFor(groupChat))); got != emailA {
+		t.Errorf("admin A's card shows email %q, want its own %q from admin B's draft", got, emailA)
+	}
+
+	// The step they are each on is their own too: A's prompt must not put B into
+	// the same step, or B's next message lands in A's wizard.
+	tap(adminA, "add_client_ch_default_email")
+	if st, ok := userStateMgr.get(chatUser{chatID: groupChat, userID: adminA}); !ok || st != "awaiting_email" {
+		t.Errorf("admin A's step = %q (set: %v), want awaiting_email", st, ok)
+	}
+	if st, ok := userStateMgr.get(chatUser{chatID: groupChat, userID: adminB}); ok {
+		t.Errorf("admin B's step = %q, want none: only the tapper's step may change", st)
+	}
+}
+
+// The wizard's typed steps arrive as messages, and that handler is a closure no
+// test can drive, so pin the key it takes there: sender, not chat.
+func TestMessageActorSeparatesAdminsInOneChat(t *testing.T) {
+	const (
+		groupChat = int64(-1001234567890)
+		adminA    = int64(8101)
+		adminB    = int64(8202)
+	)
+	t.Cleanup(userStateMgr.reset)
+
+	fromA := messageActor(telego.Message{Chat: telego.Chat{ID: groupChat}, From: &telego.User{ID: adminA}})
+	fromB := messageActor(telego.Message{Chat: telego.Chat{ID: groupChat}, From: &telego.User{ID: adminB}})
+	// A channel post carries no sender and must not land on an admin's step.
+	fromChannel := messageActor(telego.Message{Chat: telego.Chat{ID: groupChat}})
+
+	if want := (chatUser{chatID: groupChat, userID: adminA}); fromA != want {
+		t.Errorf("message from admin A keyed as %+v, want %+v", fromA, want)
+	}
+	if fromA == fromB || fromA == fromChannel || fromB == fromChannel {
+		t.Fatalf("keys collide: %+v, %+v, %+v", fromA, fromB, fromChannel)
+	}
+
+	userStateMgr.set(fromA, "awaiting_email")
+	if st, ok := userStateMgr.get(fromB); ok {
+		t.Errorf("admin B sees step %q: admin A's answer would land in B's wizard", st)
+	}
+	if st, ok := userStateMgr.get(fromChannel); ok {
+		t.Errorf("a senderless post sees step %q", st)
+	}
+	if st, ok := userStateMgr.get(fromA); !ok || st != "awaiting_email" {
+		t.Errorf("admin A's step = %q (set: %v), want awaiting_email", st, ok)
+	}
+}

internal/web/service/tgbot/tgbot_client_draft_per_chat_test.go

@@ -139,7 +139,7 @@ func TestNonWizardCallbackTakesNoDraftLock(t *testing.T) {
 	)
 	decliningServer(t)
 
-	held := addClientDrafts.forChat(heldChat)
+	held := addClientDrafts.forActor(chatUser{chatID: heldChat, userID: 1})
 	held.Lock()
 	defer held.Unlock()
 
@@ -170,7 +170,7 @@ func TestNonWizardCallbackTakesNoDraftLock(t *testing.T) {
 	tap(spareChat, false, "add_client_to 1")
 
 	addClientDrafts.mu.Lock()
-	_, stored := addClientDrafts.drafts[spareChat]
+	_, stored := addClientDrafts.drafts[chatUser{chatID: spareChat, userID: 1}]
 	addClientDrafts.mu.Unlock()
 	if stored {
 		t.Errorf("draft stored for chat %d, want none until its wizard starts", spareChat)

internal/web/service/tgbot/tgbot_delete_after_test.go

@@ -7,12 +7,12 @@ func TestDeleteMessageAfterDelayKeepsUserState(t *testing.T) {
 	t.Cleanup(userStateMgr.reset)
 
 	const chatID = int64(4242)
-	userStateMgr.set(chatID, "awaiting_comment")
+	userStateMgr.set(chatUser{chatID: chatID, userID: 1}, "awaiting_comment")
 
 	tg := &Tgbot{}
 	tg.deleteMessageAfterDelay(chatID, 1, 0)
 
-	if st, ok := userStateMgr.get(chatID); !ok || st != "awaiting_comment" {
+	if st, ok := userStateMgr.get(chatUser{chatID: chatID, userID: 1}); !ok || st != "awaiting_comment" {
 		t.Fatalf("delayed message deletion cleared the conversation state: got (%q, %v), want (%q, true)", st, ok, "awaiting_comment")
 	}
 }

internal/web/service/tgbot/tgbot_draft_render_test.go

@@ -20,8 +20,8 @@ const clientDraftTestChatID = -9001
 // Regression test: the draft is sent with ParseMode HTML, so Markdown markers
 // were rendered literally and an unescaped value could break the whole message.
 func TestClientDraftMessageRendersHTML(t *testing.T) {
-	draft := addClientDrafts.forChat(clientDraftTestChatID)
-	t.Cleanup(func() { addClientDrafts.reset(clientDraftTestChatID) })
+	draft := addClientDrafts.forActor(chatUser{chatID: clientDraftTestChatID, userID: 1})
+	t.Cleanup(func() { addClientDrafts.reset(chatUser{chatID: clientDraftTestChatID, userID: 1}) })
 
 	draft.email = "[email protected]"
 	draft.comment = "<b>promo</b> & <10 GB>"
@@ -67,10 +67,10 @@ func TestAddClientPromptsEscapeDraftValues(t *testing.T) {
 	url, texts := draftTexts(t)
 	swapTestBot(t, url)
 
-	draft := addClientDrafts.forChat(1)
+	draft := addClientDrafts.forActor(chatUser{chatID: 1, userID: 1})
 	origRunning := isRunning
 	t.Cleanup(func() {
-		addClientDrafts.reset(1)
+		addClientDrafts.reset(chatUser{chatID: 1, userID: 1})
 		isRunning = origRunning
 	})
 	isRunning = true

internal/web/service/tgbot/tgbot_router.go

@@ -82,7 +82,7 @@ func (t *Tgbot) OnReceive() {
 
 		h.HandleMessage(func(ctx *th.Context, message telego.Message) error {
 			defer recoverBotPanic()
-			userStateMgr.clear(message.Chat.ID)
+			userStateMgr.clear(messageActor(message))
 			t.SendMsgToTgbot(message.Chat.ID, t.I18nBot("tgbot.keyboardClosed"), tu.ReplyKeyboardRemove())
 			return nil
 		}, th.TextEqual(t.I18nBot("tgbot.buttons.closeKeyboard")))
@@ -95,7 +95,7 @@ func (t *Tgbot) OnReceive() {
 
 			// Use goroutine with worker pool for concurrent command processing
 			go runBotHandler(func() {
-				userStateMgr.clear(message.Chat.ID)
+				userStateMgr.clear(messageActor(message))
 				t.answerCommand(&message, message.Chat.ID, checkAdmin(message.From.ID))
 			})
 			return nil
@@ -104,7 +104,7 @@ func (t *Tgbot) OnReceive() {
 		h.HandleCallbackQuery(func(ctx *th.Context, query telego.CallbackQuery) error {
 			// Use goroutine with worker pool for concurrent callback processing
 			go runBotHandler(func() {
-				userStateMgr.clear(query.Message.GetChat().ID)
+				userStateMgr.clear(callbackActor(&query))
 				t.answerCallback(&query, checkAdmin(query.From.ID))
 			})
 			return nil
@@ -113,22 +113,23 @@ func (t *Tgbot) OnReceive() {
 		h.HandleMessage(func(ctx *th.Context, message telego.Message) error {
 			defer recoverBotPanic()
 			userStateMgr.maybePrune(time.Hour)
-			if userState, exists := userStateMgr.get(message.Chat.ID); exists {
+			actor := messageActor(message)
+			if userState, exists := userStateMgr.get(actor); exists {
 				// Only a wizard step touches the draft, so only it takes the lock.
-				draft := addClientDrafts.forChat(message.Chat.ID)
+				draft := addClientDrafts.forActor(actor)
 				draft.Lock()
 				defer draft.Unlock()
 				switch userState {
 				case "awaiting_email":
 					if draft.email == strings.TrimSpace(message.Text) {
 						t.SendMsgToTgbotDeleteAfter(message.Chat.ID, t.I18nBot("tgbot.messages.using_default_value"), 3, tu.ReplyKeyboardRemove())
-						userStateMgr.clear(message.Chat.ID)
+						userStateMgr.clear(actor)
 						return nil
 					}
 
 					draft.email = strings.TrimSpace(message.Text)
 					if t.isSingleWord(draft.email) {
-						userStateMgr.set(message.Chat.ID, "awaiting_email")
+						userStateMgr.set(actor, "awaiting_email")
 
 						cancel_btn_markup := tu.InlineKeyboard(
 							tu.InlineKeyboardRow(
@@ -139,26 +140,26 @@ func (t *Tgbot) OnReceive() {
 						t.SendMsgToTgbot(message.Chat.ID, t.I18nBot("tgbot.messages.incorrect_input"), cancel_btn_markup)
 					} else {
 						t.SendMsgToTgbotDeleteAfter(message.Chat.ID, t.I18nBot("tgbot.messages.received_email"), 3, tu.ReplyKeyboardRemove())
-						userStateMgr.clear(message.Chat.ID)
+						userStateMgr.clear(actor)
 						t.addClient(message.Chat.ID, draft, t.BuildClientDraftMessage(draft))
 					}
 				case "awaiting_comment":
 					if draft.comment == strings.TrimSpace(message.Text) {
 						t.SendMsgToTgbotDeleteAfter(message.Chat.ID, t.I18nBot("tgbot.messages.using_default_value"), 3, tu.ReplyKeyboardRemove())
-						userStateMgr.clear(message.Chat.ID)
+						userStateMgr.clear(actor)
 						return nil
 					}
 
 					draft.comment = strings.TrimSpace(message.Text)
 					t.SendMsgToTgbotDeleteAfter(message.Chat.ID, t.I18nBot("tgbot.messages.received_comment"), 3, tu.ReplyKeyboardRemove())
-					userStateMgr.clear(message.Chat.ID)
+					userStateMgr.clear(actor)
 					t.addClient(message.Chat.ID, draft, t.BuildClientDraftMessage(draft))
 				case "awaiting_tg_id":
 					input := strings.TrimSpace(message.Text)
 					if input == "" || input == "-" || strings.EqualFold(input, "none") {
 						draft.tgID = ""
 						t.SendMsgToTgbotDeleteAfter(message.Chat.ID, t.I18nBot("tgbot.messages.using_default_value"), 3, tu.ReplyKeyboardRemove())
-						userStateMgr.clear(message.Chat.ID)
+						userStateMgr.clear(actor)
 						t.addClient(message.Chat.ID, draft, t.BuildClientDraftMessage(draft))
 						return nil
 					}
@@ -173,7 +174,7 @@ func (t *Tgbot) OnReceive() {
 					}
 					draft.tgID = input
 					t.SendMsgToTgbotDeleteAfter(message.Chat.ID, t.I18nBot("tgbot.messages.userSaved"), 3, tu.ReplyKeyboardRemove())
-					userStateMgr.clear(message.Chat.ID)
+					userStateMgr.clear(actor)
 					t.addClient(message.Chat.ID, draft, t.BuildClientDraftMessage(draft))
 				}
 			} else {
@@ -315,12 +316,13 @@ func isCommandForBot(text string, username string) bool {
 // answerCallback processes callback queries from inline keyboards.
 func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool) {
 	chatId := callbackQuery.Message.GetChat().ID
+	actor := callbackActor(callbackQuery)
 
 	// Only an admin's wizard callbacks touch a draft, so only they take its lock:
 	// a report tap must not wait on a slot, a rejected chat must not be stored.
 	var draft *clientDraft
 	if isAdmin && isAddClientStep(callbackQuery.Data) {
-		draft = addClientDrafts.forChat(chatId)
+		draft = addClientDrafts.forActor(actor)
 		draft.Lock()
 		defer draft.Unlock()
 	}
@@ -1066,7 +1068,7 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 		t.SendMsgToTgbot(chatId, t.I18nBot("tgbot.answers.chooseInbound"), inbounds)
 	case "add_client_ch_default_email":
 		t.deleteMessageTgBot(chatId, callbackQuery.Message.GetMessageID())
-		userStateMgr.set(chatId, "awaiting_email")
+		userStateMgr.set(actor, "awaiting_email")
 		cancel_btn_markup := tu.InlineKeyboard(
 			tu.InlineKeyboardRow(
 				tu.InlineKeyboardButton(t.I18nBot("tgbot.buttons.use_default")).WithCallbackData("add_client_default_info"),
@@ -1076,7 +1078,7 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 		t.SendMsgToTgbot(chatId, prompt_message, cancel_btn_markup)
 	case "add_client_ch_default_comment":
 		t.deleteMessageTgBot(chatId, callbackQuery.Message.GetMessageID())
-		userStateMgr.set(chatId, "awaiting_comment")
+		userStateMgr.set(actor, "awaiting_comment")
 		cancel_btn_markup := tu.InlineKeyboard(
 			tu.InlineKeyboardRow(
 				tu.InlineKeyboardButton(t.I18nBot("tgbot.buttons.use_default")).WithCallbackData("add_client_default_info"),
@@ -1086,7 +1088,7 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 		t.SendMsgToTgbot(chatId, prompt_message, cancel_btn_markup)
 	case "add_client_ch_default_tg_id":
 		t.deleteMessageTgBot(chatId, callbackQuery.Message.GetMessageID())
-		userStateMgr.set(chatId, "awaiting_tg_id")
+		userStateMgr.set(actor, "awaiting_tg_id")
 		cancel_btn_markup := tu.InlineKeyboard(
 			tu.InlineKeyboardRow(
 				tu.InlineKeyboardButton(t.I18nBot("tgbot.buttons.use_default")).WithCallbackData("add_client_default_info"),
@@ -1189,11 +1191,11 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 	case "add_client_default_info":
 		t.deleteMessageTgBot(chatId, callbackQuery.Message.GetMessageID())
 		t.SendMsgToTgbotDeleteAfter(chatId, t.I18nBot("tgbot.messages.using_default_value"), 3, tu.ReplyKeyboardRemove())
-		userStateMgr.clear(chatId)
+		userStateMgr.clear(actor)
 		t.addClient(chatId, draft, t.BuildClientDraftMessage(draft))
 	case "add_client_cancel":
-		userStateMgr.clear(chatId)
-		addClientDrafts.reset(chatId)
+		userStateMgr.clear(actor)
+		addClientDrafts.reset(actor)
 		t.deleteMessageTgBot(chatId, callbackQuery.Message.GetMessageID())
 		t.SendMsgToTgbotDeleteAfter(chatId, t.I18nBot("tgbot.messages.cancel"), 3, tu.ReplyKeyboardRemove())
 	case "add_client_default_traffic_exp":
@@ -1235,7 +1237,7 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 			t.SendMsgToTgbot(chatId, t.I18nBot("tgbot.answers.successfulOperation"), tu.ReplyKeyboardRemove())
 			t.sendClientIndividualLinks(chatId, draft.email)
 			t.sendClientQRLinks(chatId, draft.email)
-			addClientDrafts.reset(chatId)
+			addClientDrafts.reset(actor)
 		}
 	case "add_client_submit_enable":
 		draft.enable = true
@@ -1248,7 +1250,7 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 			t.SendMsgToTgbot(chatId, t.I18nBot("tgbot.answers.successfulOperation"), tu.ReplyKeyboardRemove())
 			t.sendClientIndividualLinks(chatId, draft.email)
 			t.sendClientQRLinks(chatId, draft.email)
-			addClientDrafts.reset(chatId)
+			addClientDrafts.reset(actor)
 		}
 	case "reset_all_traffics_cancel":
 		t.deleteMessageTgBot(chatId, callbackQuery.Message.GetMessageID())