fix(admin): 渠道 personaId 改为三态语义(省略=保留 / ""=解绑 / id=绑定)——修「保存其它配置即静默清空绑定」
跨端语义相反:后端 5969bba 实现的是「省略/\"\"=解绑」,而管理面板只在用户改动下拉时才发该键 ⇒ owner 打开已绑账号、不动下拉、保存其它配置就会静默清空渠道角色绑定。
- 请求 DTO 改 *string 区分「省略」与「显式空串」:省略 → 保留现状;显式 "" → 解绑;非空 id → 绑定(未知角色 400);沿用读-改-写 + bridge.Reload 热更新 + 审计
- GET /admin/channels 继续回显 account.personaId(已确认)
- Docs/api-client 三态语义写清(省略=保留是本次的关键修正)
- 测试:省略保留 / 显式空串解绑 / id 绑定 / 未知 id 400 + 变异验证(把省略改回解绑 → 断言 FAIL)
This commit is contained in:
1 parent
5969bba3bd
commit
f9430dee81
3 files changed
+236
-52
No files matched your search
@@ -1,8 +1,11 @@
|
||||
package gateway
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
@@ -18,10 +21,96 @@ import (
|
||||
// existing read-modify-write + bridge.Reload path, an unknown role is refused
|
||||
// instead of binding a role that cannot answer, and the list endpoint reports the
|
||||
// binding back.
|
||||
//
|
||||
// personaId is three-state on the wire, because the panel sends the key only when
|
||||
// the owner actually touched the role select:
|
||||
//
|
||||
// key absent → keep the stored binding
|
||||
// explicit "" → clear the binding
|
||||
// non-empty id → bind that role (unknown id → 400)
|
||||
//
|
||||
// The absent-key case is the one that silently dropped bindings, so it is asserted
|
||||
// with a literal JSON body: a map[string]any would still make the assertion pass if
|
||||
// the DTO had picked up a value from somewhere else.
|
||||
|
||||
// TestAdminSaveChannelConfigPersonaID saves a channel role, asserts it reached
|
||||
// channels.json and the list wire shape, then clears it.
|
||||
func TestAdminSaveChannelConfigPersonaID(t *testing.T) {
|
||||
// putChannelConfigRaw posts a literal JSON body so a test can pin the exact wire
|
||||
// shape (notably: no personaId key at all).
|
||||
func putChannelConfigRaw(t *testing.T, ts *httptest.Server, platform, raw, token string) (*http.Response, []byte) {
|
||||
t.Helper()
|
||||
req, err := http.NewRequest(http.MethodPut,
|
||||
ts.URL+"/api/v1/admin/channels/"+platform+"/config", bytes.NewReader([]byte(raw)))
|
||||
if err != nil {
|
||||
t.Fatalf("new request: %v", err)
|
||||
}
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
t.Fatalf("do request: %v", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
b, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
t.Fatalf("read response: %v", err)
|
||||
}
|
||||
if resp.StatusCode != http.StatusCreated {
|
||||
t.Fatalf("PUT %s status = %d, want 201 (body=%s)", platform, resp.StatusCode, b)
|
||||
}
|
||||
return resp, b
|
||||
}
|
||||
|
||||
// storedPersonaID reads the binding channels.json holds for one account.
|
||||
func storedPersonaID(t *testing.T, path, platform, accountID string) string {
|
||||
t.Helper()
|
||||
accounts, warn := runtimeconfig.LoadChannels(path, nil)
|
||||
if warn != nil {
|
||||
t.Fatalf("load channels warn = %v", warn)
|
||||
}
|
||||
for _, acct := range accounts[platform] {
|
||||
if acct.ID == accountID {
|
||||
return acct.PersonaID
|
||||
}
|
||||
}
|
||||
t.Fatalf("account %s/%s not found in channels.json: %+v", platform, accountID, accounts[platform])
|
||||
return ""
|
||||
}
|
||||
|
||||
// listedPersonaID is the personaId GET /admin/channels reports for one account, i.e.
|
||||
// what the panel reads back to render the role select.
|
||||
func listedPersonaID(t *testing.T, ts *httptest.Server, token, platform, accountID string) (string, []byte) {
|
||||
t.Helper()
|
||||
resp, body := doReq(t, http.MethodGet, ts.URL+"/api/v1/admin/channels", nil, token)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("get status = %d, want 200 (body=%s)", resp.StatusCode, body)
|
||||
}
|
||||
var listed []struct {
|
||||
Name string `json:"name"`
|
||||
Accounts []struct {
|
||||
ID string `json:"id"`
|
||||
PersonaID string `json:"personaId"`
|
||||
} `json:"accounts"`
|
||||
}
|
||||
if err := json.Unmarshal(decodeBody(t, body)["data"], &listed); err != nil {
|
||||
t.Fatalf("decode channels list: %v (body=%s)", err, body)
|
||||
}
|
||||
for _, entry := range listed {
|
||||
if entry.Name != platform {
|
||||
continue
|
||||
}
|
||||
for _, acct := range entry.Accounts {
|
||||
if acct.ID == accountID {
|
||||
return acct.PersonaID, body
|
||||
}
|
||||
}
|
||||
}
|
||||
t.Fatalf("account %s/%s not in GET /admin/channels (body=%s)", platform, accountID, body)
|
||||
return "", body
|
||||
}
|
||||
|
||||
// TestAdminSaveChannelConfigPersonaIDBindsAndEchoes binds a channel role and
|
||||
// asserts it reached channels.json, that the list endpoint echoes it back for the
|
||||
// panel to render, and that the hot-reload path ran.
|
||||
func TestAdminSaveChannelConfigPersonaIDBindsAndEchoes(t *testing.T) {
|
||||
t.Parallel()
|
||||
path := filepath.Join(t.TempDir(), "channels.json")
|
||||
store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{}}
|
||||
@@ -33,51 +122,99 @@ func TestAdminSaveChannelConfigPersonaID(t *testing.T) {
|
||||
if resp.StatusCode != http.StatusCreated {
|
||||
t.Fatalf("save status = %d, want 201 (body=%s)", resp.StatusCode, body)
|
||||
}
|
||||
accounts, warn := runtimeconfig.LoadChannels(path, nil)
|
||||
if warn != nil {
|
||||
t.Fatalf("load channels warn = %v", warn)
|
||||
}
|
||||
if len(accounts["telegram"]) != 1 || accounts["telegram"][0].PersonaID != "yuki" {
|
||||
t.Fatalf("channels.json telegram = %+v, want personaId yuki", accounts["telegram"])
|
||||
if got := storedPersonaID(t, path, "telegram", "conf0"); got != "yuki" {
|
||||
t.Fatalf("channels.json telegram.conf0 personaId = %q, want yuki", got)
|
||||
}
|
||||
if len(store.reloaded) != 1 || store.reloaded[0] != "telegram" {
|
||||
t.Fatalf("reloaded = %v, want [telegram] (the panel's hot-reload path)", store.reloaded)
|
||||
}
|
||||
|
||||
// The list endpoint reports the binding so the panel can render it.
|
||||
var listed []struct {
|
||||
Name string `json:"name"`
|
||||
Accounts []struct {
|
||||
ID string `json:"id"`
|
||||
PersonaID string `json:"personaId"`
|
||||
} `json:"accounts"`
|
||||
echoed, listBody := listedPersonaID(t, ts, tok, "telegram", "conf0")
|
||||
if echoed != "yuki" {
|
||||
t.Fatalf("GET /admin/channels telegram.conf0 personaId = %q, want yuki (body=%s)", echoed, listBody)
|
||||
}
|
||||
_, gBody := doReq(t, http.MethodGet, ts.URL+"/api/v1/admin/channels", nil, tok)
|
||||
if err := json.Unmarshal(decodeBody(t, gBody)["data"], &listed); err != nil {
|
||||
t.Fatalf("decode channels list: %v (body=%s)", err, gBody)
|
||||
t.Logf("GET /api/v1/admin/channels echoes the binding: %s", listBody)
|
||||
}
|
||||
|
||||
// TestAdminSaveChannelConfigPersonaIDKeptWhenOmitted is the regression for the
|
||||
// silent data loss: the panel omits personaId unless the owner touched the role
|
||||
// select, so saving another field must leave the stored binding alone.
|
||||
func TestAdminSaveChannelConfigPersonaIDKeptWhenOmitted(t *testing.T) {
|
||||
t.Parallel()
|
||||
path := filepath.Join(t.TempDir(), "channels.json")
|
||||
store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{
|
||||
"telegram": {{ID: "conf0", Config: map[string]any{"bot_token": "old-tok"}, PersonaID: "yuki"}},
|
||||
}}
|
||||
_, ts, jwt := buildEnv(t, cfgDefault(), Deps{Channels: store, Personas: personaTestRegistry(t)})
|
||||
tok := signAny(t, jwt, "acct", auth.RoleOwner)
|
||||
|
||||
// The wire body carries the new token and no personaId key at all — exactly
|
||||
// what an untouched role select produces.
|
||||
_, body := putChannelConfigRaw(t, ts,
|
||||
"telegram", `{"id":"conf0","token":"new-tok","groupMsgPerMin":5}`, tok)
|
||||
|
||||
if got := storedPersonaID(t, path, "telegram", "conf0"); got != "yuki" {
|
||||
t.Fatalf("omitted personaId cleared the binding: channels.json personaId = %q, want yuki (resp=%s)", got, body)
|
||||
}
|
||||
var accts []struct {
|
||||
ID string `json:"id"`
|
||||
PersonaID string `json:"personaId"`
|
||||
accounts, _ := runtimeconfig.LoadChannels(path, nil)
|
||||
if accounts["telegram"][0].Config["bot_token"] != "new-tok" || accounts["telegram"][0].GroupMsgPerMin != 5 {
|
||||
t.Fatalf("the other fields were not saved: %+v", accounts["telegram"][0])
|
||||
}
|
||||
for _, entry := range listed {
|
||||
if entry.Name == "telegram" {
|
||||
accts = entry.Accounts
|
||||
}
|
||||
}
|
||||
if len(accts) != 1 || accts[0].PersonaID != "yuki" {
|
||||
t.Fatalf("GET /admin/channels telegram accounts = %+v, want personaId yuki (body=%s)", accts, gBody)
|
||||
if echoed, listBody := listedPersonaID(t, ts, tok, "telegram", "conf0"); echoed != "yuki" {
|
||||
t.Fatalf("GET /admin/channels personaId = %q, want yuki (body=%s)", echoed, listBody)
|
||||
}
|
||||
|
||||
// Saving the same account without a personaId unbinds it.
|
||||
resp, body = doReq(t, http.MethodPut, ts.URL+"/api/v1/admin/channels/telegram/config",
|
||||
map[string]any{"id": "conf0", "token": "bot-tok"}, tok)
|
||||
if resp.StatusCode != http.StatusCreated {
|
||||
t.Fatalf("unbind status = %d, want 201 (body=%s)", resp.StatusCode, body)
|
||||
// An explicit JSON null is the same as an absent key: the DTO cannot tell them
|
||||
// apart, and neither means "clear".
|
||||
putChannelConfigRaw(t, ts, "telegram", `{"id":"conf0","token":"newer-tok","personaId":null}`, tok)
|
||||
if got := storedPersonaID(t, path, "telegram", "conf0"); got != "yuki" {
|
||||
t.Fatalf("null personaId cleared the binding: personaId = %q, want yuki", got)
|
||||
}
|
||||
accounts, _ = runtimeconfig.LoadChannels(path, nil)
|
||||
if len(accounts["telegram"]) != 1 || accounts["telegram"][0].PersonaID != "" {
|
||||
t.Fatalf("after unbind = %+v, want personaId cleared", accounts["telegram"])
|
||||
}
|
||||
|
||||
// TestAdminSaveChannelConfigPersonaIDClearedOnExplicitEmpty asserts that clearing
|
||||
// the binding still works, but only when the panel says so with an explicit "".
|
||||
func TestAdminSaveChannelConfigPersonaIDClearedOnExplicitEmpty(t *testing.T) {
|
||||
t.Parallel()
|
||||
path := filepath.Join(t.TempDir(), "channels.json")
|
||||
store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{
|
||||
"telegram": {{ID: "conf0", Config: map[string]any{"bot_token": "old-tok"}, PersonaID: "yuki"}},
|
||||
}}
|
||||
_, ts, jwt := buildEnv(t, cfgDefault(), Deps{Channels: store, Personas: personaTestRegistry(t)})
|
||||
tok := signAny(t, jwt, "acct", auth.RoleOwner)
|
||||
|
||||
putChannelConfigRaw(t, ts, "telegram", `{"id":"conf0","token":"new-tok","personaId":""}`, tok)
|
||||
|
||||
if got := storedPersonaID(t, path, "telegram", "conf0"); got != "" {
|
||||
t.Fatalf("explicit \"\" personaId = %q, want the binding cleared", got)
|
||||
}
|
||||
if echoed, listBody := listedPersonaID(t, ts, tok, "telegram", "conf0"); echoed != "" {
|
||||
t.Fatalf("GET /admin/channels personaId = %q, want empty (body=%s)", echoed, listBody)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminSaveChannelConfigPersonaIDKeptPerAccount pins the read-modify-write to
|
||||
// the account being saved: an omitted personaId on one account must inherit that
|
||||
// account's binding, not a sibling's.
|
||||
func TestAdminSaveChannelConfigPersonaIDKeptPerAccount(t *testing.T) {
|
||||
t.Parallel()
|
||||
path := filepath.Join(t.TempDir(), "channels.json")
|
||||
store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{
|
||||
"telegram": {
|
||||
{ID: "conf0", Config: map[string]any{"bot_token": "a"}, PersonaID: "yuki"},
|
||||
{ID: "conf1", Config: map[string]any{"bot_token": "b"}},
|
||||
},
|
||||
}}
|
||||
_, ts, jwt := buildEnv(t, cfgDefault(), Deps{Channels: store, Personas: personaTestRegistry(t)})
|
||||
tok := signAny(t, jwt, "acct", auth.RoleOwner)
|
||||
|
||||
putChannelConfigRaw(t, ts, "telegram", `{"id":"conf1","token":"b2"}`, tok)
|
||||
|
||||
if got := storedPersonaID(t, path, "telegram", "conf0"); got != "yuki" {
|
||||
t.Fatalf("sibling account conf0 personaId = %q, want yuki untouched", got)
|
||||
}
|
||||
if got := storedPersonaID(t, path, "telegram", "conf1"); got != "" {
|
||||
t.Fatalf("conf1 personaId = %q, want empty (it never had a binding)", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -87,7 +224,9 @@ func TestAdminSaveChannelConfigPersonaID(t *testing.T) {
|
||||
func TestAdminSaveChannelConfigRejectsUnknownPersonaID(t *testing.T) {
|
||||
t.Parallel()
|
||||
path := filepath.Join(t.TempDir(), "channels.json")
|
||||
store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{}}
|
||||
store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{
|
||||
"telegram": {{ID: "conf0", Config: map[string]any{"bot_token": "old-tok"}, PersonaID: "yuki"}},
|
||||
}}
|
||||
_, ts, jwt := buildEnv(t, cfgDefault(), Deps{Channels: store, Personas: personaTestRegistry(t)})
|
||||
tok := signAny(t, jwt, "acct", auth.RoleOwner)
|
||||
|
||||
|
||||
@@ -75,17 +75,22 @@ type channelConfigRequest struct {
|
||||
GroupMsgPerMin int `json:"groupMsgPerMin"`
|
||||
MsgPerMin int `json:"msgPerMin"`
|
||||
MaxMessages int `json:"maxMessages"`
|
||||
// PersonaID is the role this account answers as (design/22 §2 方案 b). An empty
|
||||
// value leaves the account unbound (the session binding or the global default
|
||||
// applies); an unknown id is refused with 400 so a save can never bind a role
|
||||
// that cannot answer.
|
||||
PersonaID string `json:"personaId"`
|
||||
// PersonaID is the role this account answers as (design/22 §2 方案 b). It is a
|
||||
// pointer because the wire contract is three-state and a plain string cannot
|
||||
// tell an absent key from an explicit "": absent keeps the stored binding,
|
||||
// "" clears it, a non-empty id binds it. An unknown id is refused with 400 so a
|
||||
// save can never bind a role that cannot answer.
|
||||
PersonaID *string `json:"personaId"`
|
||||
}
|
||||
|
||||
// adminSaveChannelConfig persists one account's config for a platform into
|
||||
// channels.json (Upsert by id) and hot-reloads that platform's adapters. The
|
||||
// response carries the channels.json path and the account id so the panel can
|
||||
// address the saved account (for a later edit/delete).
|
||||
//
|
||||
// personaId is three-state (see channelPersonaBinding): the panel only sends the
|
||||
// key when the owner touched the role select, so an absent key must keep the
|
||||
// stored binding rather than silently unbind the account.
|
||||
func (s *Server) adminSaveChannelConfig(w http.ResponseWriter, r *http.Request) {
|
||||
platform := strings.TrimSpace(r.PathValue("platform"))
|
||||
if platform == "" {
|
||||
@@ -104,11 +109,15 @@ func (s *Server) adminSaveChannelConfig(w http.ResponseWriter, r *http.Request)
|
||||
return
|
||||
}
|
||||
// A channel-level role must exist in the persona library, or the account would
|
||||
// be bound to a role that cannot answer (design/22 §2 方案 b). An omitted/empty
|
||||
// personaId is always accepted: it means "no channel binding".
|
||||
if !s.requireKnownPersona(w, req.PersonaID) {
|
||||
s.auditClaims(r, "channel.save", "error")
|
||||
return
|
||||
// be bound to a role that cannot answer (design/22 §2 方案 b). Only a value the
|
||||
// panel actually sent is validated; an absent key (nil) inherits the stored
|
||||
// binding, which was validated when it was written. An explicit "" is always
|
||||
// accepted: it means "clear the channel binding".
|
||||
if req.PersonaID != nil {
|
||||
if !s.requireKnownPersona(w, strings.TrimSpace(*req.PersonaID)) {
|
||||
s.auditClaims(r, "channel.save", "error")
|
||||
return
|
||||
}
|
||||
}
|
||||
cfg := channelConfigFromRequest(req)
|
||||
// weixin-ilink may be saved with an empty config: the panel creates the account
|
||||
@@ -131,7 +140,7 @@ func (s *Server) adminSaveChannelConfig(w http.ResponseWriter, r *http.Request)
|
||||
GroupMsgPerMin: req.GroupMsgPerMin,
|
||||
MsgPerMin: req.MsgPerMin,
|
||||
MaxMessages: req.MaxMessages,
|
||||
PersonaID: strings.TrimSpace(req.PersonaID),
|
||||
PersonaID: channelPersonaBinding(req.PersonaID, existingChannelAccount(accounts[platform], req.ID)),
|
||||
})
|
||||
if err := runtimeconfig.SaveChannels(s.channels.Path(), updated); err != nil {
|
||||
s.auditClaims(r, "channel.save", "error")
|
||||
@@ -244,6 +253,41 @@ func channelConfigFromRequest(req channelConfigRequest) map[string]any {
|
||||
return cfg
|
||||
}
|
||||
|
||||
// channelPersonaBinding resolves the three-state personaId contract of
|
||||
// PUT /admin/channels/{platform}/config:
|
||||
//
|
||||
// - the key is absent (nil) → keep the account's stored binding;
|
||||
// - the key is an explicit "" → clear the binding (an explicit unbind);
|
||||
// - the key is a non-empty id → bind that role.
|
||||
//
|
||||
// The panel omits the key unless the owner touched the role select, so resolving
|
||||
// an absent key from the stored account is what keeps a plain config edit from
|
||||
// silently dropping a binding. A save that creates a new account (no stored
|
||||
// account to inherit from) resolves to "no binding".
|
||||
func channelPersonaBinding(sent *string, stored config.ChannelAccount) string {
|
||||
if sent == nil {
|
||||
return strings.TrimSpace(stored.PersonaID)
|
||||
}
|
||||
return strings.TrimSpace(*sent)
|
||||
}
|
||||
|
||||
// existingChannelAccount returns the stored account a save would replace (same
|
||||
// platform, same id) so the write can inherit the fields the panel did not send.
|
||||
// An empty id means the save creates a new account, so there is nothing to
|
||||
// inherit.
|
||||
func existingChannelAccount(list []config.ChannelAccount, id string) config.ChannelAccount {
|
||||
id = strings.TrimSpace(id)
|
||||
if id == "" {
|
||||
return config.ChannelAccount{}
|
||||
}
|
||||
for _, acct := range list {
|
||||
if acct.ID == id {
|
||||
return acct
|
||||
}
|
||||
}
|
||||
return config.ChannelAccount{}
|
||||
}
|
||||
|
||||
// savedAccountID returns the account id that a save produced — the caller-supplied
|
||||
// one when present, otherwise the id runtimeconfig.Upsert generated (which it
|
||||
// appends last for a brand-new account).
|
||||
|
||||
Reference in new issue
Block a user