diff --git a/apps/docs/content/docs/slack-bot-integration.ja.mdx b/apps/docs/content/docs/slack-bot-integration.ja.mdx index b05dd5348..da6f8b163 100644 --- a/apps/docs/content/docs/slack-bot-integration.ja.mdx +++ b/apps/docs/content/docs/slack-bot-integration.ja.mdx @@ -134,6 +134,8 @@ app-level トークンは Socket Mode 接続を認可します。これはコン 初めて Bot を @ メンションするか DM すると、Bot は **アカウントを紐づける** プロンプトで返信します。リンクをタップして Multica にサインインすると、あなたの Slack アイデンティティがあなたの Multica メンバーシップに紐づきます——これによって、エージェントがあなたとして振る舞えるようになります(たとえば `/issue` はあなたの名義でイシューを起票します)。このリンクは使い切りで、約 15 分で失効します。新しいものが必要なら、もう一度 Bot にメッセージを送るだけです。 +**紐づけは Slack ワークスペースごとに一度だけです。** 同じ Multica ワークスペースが 1 つの Slack ワークスペースで複数の Bot を運用している場合(エージェントごとに 1 つのアプリ)、最初の Bot で紐づけを済ませれば、残りはその紐づけを自動的に再利用し、再びプロンプトは出ません。(再度の紐づけが必要なのは、*別の* Slack ワークスペースにある Bot、または *別の* Multica ワークスペースに接続された Bot だけです。) + Bot を使えるのは **ワークスペースのメンバー** だけです。メンバーでない場合や、アイデンティティの紐づけをスキップした場合、Bot は実行されません——あなたのメッセージは破棄されます(内容は保存せず、監査のために記録されます)。 diff --git a/apps/docs/content/docs/slack-bot-integration.ko.mdx b/apps/docs/content/docs/slack-bot-integration.ko.mdx index ca8bf194f..f20560584 100644 --- a/apps/docs/content/docs/slack-bot-integration.ko.mdx +++ b/apps/docs/content/docs/slack-bot-integration.ko.mdx @@ -134,6 +134,8 @@ app-level token은 Socket Mode 연결을 인가합니다. 콘솔에서만 생성 봇을 처음 `@`로 멘션하거나 DM하면, **계정을 연결하라**는 안내로 답합니다. 링크를 탭하고 Multica에 로그인하면, 당신의 Slack 신원이 Multica 멤버십에 바인딩됩니다 — 바로 이 단계가 에이전트로 하여금 당신을 대신해 행동하게 합니다(예: `/issue`는 당신 이름으로 이슈를 생성합니다). 이 링크는 일회용이며 약 15분 후에 만료됩니다. 새 링크가 필요하면 봇에게 다시 메시지를 보내세요. +**연결은 Slack 워크스페이스당 한 번만 하면 됩니다.** 같은 Multica 워크스페이스가 하나의 Slack 워크스페이스에서 여러 봇을 운영하는 경우(에이전트마다 앱 하나), 첫 봇에서 연결을 마치면 나머지는 그 연결을 자동으로 재사용하므로 다시 안내가 뜨지 않습니다. (다시 연결해야 하는 경우는 *다른* Slack 워크스페이스에 있는 봇이거나, *다른* Multica 워크스페이스에 연결된 봇일 때뿐입니다.) + **워크스페이스 멤버**만 봇을 사용할 수 있습니다. 멤버가 아니거나 신원 연결을 건너뛰면 봇은 실행되지 않으며, 메시지는 폐기됩니다(감사 목적으로 기록되며, 내용은 저장하지 않습니다). diff --git a/apps/docs/content/docs/slack-bot-integration.mdx b/apps/docs/content/docs/slack-bot-integration.mdx index b1a389820..162e8a4ff 100644 --- a/apps/docs/content/docs/slack-bot-integration.mdx +++ b/apps/docs/content/docs/slack-bot-integration.mdx @@ -134,6 +134,8 @@ Setting this up for **multiple agents**? Repeat the whole flow once per agent The first time you @-mention or DM the bot, it replies with a **link your account** prompt. Tap the link, sign in to Multica, and your Slack identity is bound to your Multica membership — this is what lets the agent act as you (e.g. `/issue` files under your name). The link is single-use and expires in about 15 minutes; just message the bot again for a fresh one. +You only link **once per Slack workspace**. If the same Multica workspace runs several bots in one Slack workspace (one app per agent), the first bot you link teaches the rest: messaging a second bot reuses that link automatically, no re-prompt. (Linking again is only needed for a bot in a *different* Slack workspace, or a bot connected to a *different* Multica workspace.) + Only **members of the workspace** can use the bot. If you aren't a member, or you skip the identity link, the bot won't run — your message is dropped (recorded for audit, without its contents). diff --git a/apps/docs/content/docs/slack-bot-integration.zh.mdx b/apps/docs/content/docs/slack-bot-integration.zh.mdx index 20e8606de..d5165e6ca 100644 --- a/apps/docs/content/docs/slack-bot-integration.zh.mdx +++ b/apps/docs/content/docs/slack-bot-integration.zh.mdx @@ -134,6 +134,8 @@ App-level token 用来授权 Socket Mode 连接。它只能在控制台里创建 第一次 @ 或私聊 Bot 时,它会回一条 **绑定你的账号** 提示。点开链接、登录 Multica,你的 Slack 身份就会绑定到你的 Multica 成员身份——正是这一步让智能体能以你的身份行事(比如 `/issue` 会把 issue 记在你名下)。这个链接是一次性的,大约 15 分钟后过期;再给 Bot 发条消息就能拿到一个新的。 +**每个 Slack workspace 只需绑定一次。** 如果同一个 Multica 工作区在同一个 Slack workspace 里跑了多个 Bot(每个智能体一个 App),你在第一个 Bot 上完成绑定后,其余的会自动复用这次绑定,不会再次弹出提示。(只有在*另一个* Slack workspace 里的 Bot、或连到*另一个* Multica 工作区的 Bot,才需要重新绑定。) + 只有**工作区成员**才能使用 Bot。如果你不是成员,或者跳过了身份绑定,Bot 不会运行——你的消息会被丢弃(仅出于审计目的记录,不保存消息内容)。 diff --git a/server/internal/integrations/slack/identity_reuse_test.go b/server/internal/integrations/slack/identity_reuse_test.go new file mode 100644 index 000000000..e24dcdbc0 --- /dev/null +++ b/server/internal/integrations/slack/identity_reuse_test.go @@ -0,0 +1,159 @@ +package slack + +import ( + "context" + "encoding/json" + "errors" + "testing" + + "github.com/jackc/pgx/v5" + + "github.com/multica-ai/multica/server/internal/integrations/channel" + "github.com/multica-ai/multica/server/internal/integrations/channel/engine" + db "github.com/multica-ai/multica/server/pkg/db/generated" +) + +// fakeIdentityQueries implements identityQueries so the cross-installation +// account-link reuse path (MUL-3911) is exercised without a database. +type fakeIdentityQueries struct { + binding db.ChannelUserBinding + bindErr error + reusable db.ChannelUserBinding + reusableErr error + memberErr error + createErr error + + findCalls int + findWith db.FindReusableChannelUserBindingParams + createCalls int + createWith db.CreateChannelUserBindingParams +} + +func (f *fakeIdentityQueries) GetChannelUserBindingByUserID(_ context.Context, _ db.GetChannelUserBindingByUserIDParams) (db.ChannelUserBinding, error) { + return f.binding, f.bindErr +} + +func (f *fakeIdentityQueries) FindReusableChannelUserBinding(_ context.Context, arg db.FindReusableChannelUserBindingParams) (db.ChannelUserBinding, error) { + f.findCalls++ + f.findWith = arg + return f.reusable, f.reusableErr +} + +func (f *fakeIdentityQueries) GetMemberByUserAndWorkspace(_ context.Context, _ db.GetMemberByUserAndWorkspaceParams) (db.Member, error) { + return db.Member{}, f.memberErr +} + +func (f *fakeIdentityQueries) CreateChannelUserBinding(_ context.Context, arg db.CreateChannelUserBindingParams) (db.ChannelUserBinding, error) { + f.createCalls++ + f.createWith = arg + return db.ChannelUserBinding{}, f.createErr +} + +// TestResolveSenderReuse covers the identity resolver's decision to reuse an +// existing account link across installations of the same Slack team + Multica +// workspace, instead of re-prompting the user for every new Slack app. +func TestResolveSenderReuse(t *testing.T) { + const senderID = "U123" + wsID := slashTestUUID(0x11) + instB := slashTestUUID(0xBB) // the installation the message arrives on + instA := slashTestUUID(0xAA) // the installation the user already linked + userID := slashTestUUID(0x77) + + // inst builds the ResolvedInstallation the message routes to; teamID is what + // its stored config carries (empty = a legacy install with no recorded team). + inst := func(teamID string) engine.ResolvedInstallation { + cfg, _ := json.Marshal(installConfig{AppID: "A_APPB", TeamID: teamID}) + return engine.ResolvedInstallation{ + ID: instB, + WorkspaceID: wsID, + Platform: db.ChannelInstallation{ID: instB, WorkspaceID: wsID, Config: cfg}, + } + } + msg := inbound(channel.ChatTypeP2P, "D1", "", "1.0") + msg.Source.SenderID = senderID + + t.Run("direct binding resolves without a reuse lookup or write", func(t *testing.T) { + f := &fakeIdentityQueries{binding: db.ChannelUserBinding{MulticaUserID: userID}} + got, err := (&identityResolver{q: f}).ResolveSender(context.Background(), inst("T1"), msg) + if err != nil { + t.Fatalf("ResolveSender err = %v", err) + } + if got.UserID != userID { + t.Errorf("UserID = %v, want %v", got.UserID, userID) + } + if f.findCalls != 0 || f.createCalls != 0 { + t.Errorf("directly-bound sender must not trigger reuse (find=%d create=%d)", f.findCalls, f.createCalls) + } + }) + + t.Run("unlinked sender reuses a same-team link and materializes it", func(t *testing.T) { + f := &fakeIdentityQueries{ + bindErr: pgx.ErrNoRows, + reusable: db.ChannelUserBinding{MulticaUserID: userID, InstallationID: instA}, + } + got, err := (&identityResolver{q: f}).ResolveSender(context.Background(), inst("T1"), msg) + if err != nil { + t.Fatalf("ResolveSender err = %v", err) + } + if got.UserID != userID { + t.Errorf("UserID = %v, want reused %v", got.UserID, userID) + } + if f.findCalls != 1 { + t.Fatalf("reuse lookup must run exactly once, ran %d", f.findCalls) + } + if f.findWith.TeamID != "T1" || f.findWith.ChannelUserID != senderID || f.findWith.ChannelType != string(TypeSlack) || f.findWith.WorkspaceID != wsID { + t.Errorf("reuse lookup args = %+v", f.findWith) + } + if f.createCalls != 1 { + t.Fatalf("reused link must be materialized on THIS installation, create ran %d", f.createCalls) + } + if f.createWith.InstallationID != instB || f.createWith.MulticaUserID != userID || f.createWith.ChannelUserID != senderID { + t.Errorf("materialized binding args = %+v (want install=%v user=%v sender=%q)", f.createWith, instB, userID, senderID) + } + }) + + t.Run("no direct binding and nothing to reuse prompts a link", func(t *testing.T) { + f := &fakeIdentityQueries{bindErr: pgx.ErrNoRows, reusableErr: pgx.ErrNoRows} + _, err := (&identityResolver{q: f}).ResolveSender(context.Background(), inst("T1"), msg) + if !errors.Is(err, engine.ErrSenderUnbound) { + t.Fatalf("err = %v, want ErrSenderUnbound", err) + } + if f.createCalls != 0 { + t.Errorf("nothing to reuse must not write a binding") + } + }) + + t.Run("reusable link whose user left the workspace prompts a fresh link", func(t *testing.T) { + f := &fakeIdentityQueries{ + bindErr: pgx.ErrNoRows, + reusable: db.ChannelUserBinding{MulticaUserID: userID, InstallationID: instA}, + memberErr: pgx.ErrNoRows, + } + _, err := (&identityResolver{q: f}).ResolveSender(context.Background(), inst("T1"), msg) + if !errors.Is(err, engine.ErrSenderUnbound) { + t.Fatalf("err = %v, want ErrSenderUnbound (fresh link, not not-member)", err) + } + if f.createCalls != 0 { + t.Errorf("must not materialize a binding for a non-member") + } + }) + + t.Run("legacy installation with no team never attempts reuse", func(t *testing.T) { + f := &fakeIdentityQueries{bindErr: pgx.ErrNoRows} + _, err := (&identityResolver{q: f}).ResolveSender(context.Background(), inst(""), msg) + if !errors.Is(err, engine.ErrSenderUnbound) { + t.Fatalf("err = %v, want ErrSenderUnbound", err) + } + if f.findCalls != 0 { + t.Errorf("an install with no recorded team must not attempt cross-app reuse") + } + }) + + t.Run("directly-bound non-member surfaces not-member", func(t *testing.T) { + f := &fakeIdentityQueries{binding: db.ChannelUserBinding{MulticaUserID: userID}, memberErr: pgx.ErrNoRows} + _, err := (&identityResolver{q: f}).ResolveSender(context.Background(), inst("T1"), msg) + if !errors.Is(err, engine.ErrSenderNotMember) { + t.Fatalf("err = %v, want ErrSenderNotMember", err) + } + }) +} diff --git a/server/internal/integrations/slack/resolvers.go b/server/internal/integrations/slack/resolvers.go index 1c5d298f4..fe3f95ec0 100644 --- a/server/internal/integrations/slack/resolvers.go +++ b/server/internal/integrations/slack/resolvers.go @@ -120,6 +120,16 @@ func nullText(s string) pgtype.Text { return pgtype.Text{String: s, Valid: true} } +// installTeamID reads the real Slack team id from a stored installation config, +// or "" if absent/undecodable. Unlike decodeCredentials / DecodePublicConfig it +// does NOT fall back to app_id: team routing and identity reuse must match the +// actual Slack workspace, and app_id != team_id for BYO apps. +func installTeamID(installConfigJSON json.RawMessage) string { + var cfg installConfig + _ = json.Unmarshal(installConfigJSON, &cfg) + return cfg.TeamID +} + // installationServesTeam reports whether an installation (its stored config) may // serve events from eventTeamID. Inbound routing keys on api_app_id, which // identifies the Slack APP, not the Slack workspace: a BYO app distributed / @@ -127,12 +137,8 @@ func nullText(s string) pgtype.Text { // So we additionally require the event's team to match the team the installed // bot belongs to. An installation with no recorded team (legacy) is permissive. func installationServesTeam(installConfigJSON json.RawMessage, eventTeamID string) bool { - // Read team_id directly (NOT via DecodePublicConfig, which falls back to - // app_id when team_id is absent — a hosted-era convenience that would defeat - // this check for BYO where app_id != team_id). - var cfg installConfig - _ = json.Unmarshal(installConfigJSON, &cfg) - return cfg.TeamID == "" || cfg.TeamID == eventTeamID + teamID := installTeamID(installConfigJSON) + return teamID == "" || teamID == eventTeamID } // ---- installation routing ---- @@ -173,32 +179,106 @@ func (r *installationResolver) ResolveInstallation(ctx context.Context, msg chan // ---- identity ---- -type identityResolver struct{ q *db.Queries } +// identityQueries is the slice of generated queries the identityResolver needs. +// It is an interface (not *db.Queries) so the cross-installation reuse path is +// unit-tested with fakes, mirroring slashQueries. *db.Queries satisfies it. +type identityQueries interface { + GetChannelUserBindingByUserID(ctx context.Context, arg db.GetChannelUserBindingByUserIDParams) (db.ChannelUserBinding, error) + FindReusableChannelUserBinding(ctx context.Context, arg db.FindReusableChannelUserBindingParams) (db.ChannelUserBinding, error) + GetMemberByUserAndWorkspace(ctx context.Context, arg db.GetMemberByUserAndWorkspaceParams) (db.Member, error) + CreateChannelUserBinding(ctx context.Context, arg db.CreateChannelUserBindingParams) (db.ChannelUserBinding, error) +} + +type identityResolver struct{ q identityQueries } func (r *identityResolver) ResolveSender(ctx context.Context, inst engine.ResolvedInstallation, msg channel.InboundMessage) (engine.ResolvedIdentity, error) { + senderID := msg.Source.SenderID binding, err := r.q.GetChannelUserBindingByUserID(ctx, db.GetChannelUserBindingByUserIDParams{ InstallationID: inst.ID, - ChannelUserID: msg.Source.SenderID, + ChannelUserID: senderID, }) + reused := false if err != nil { - if errors.Is(err, pgx.ErrNoRows) { + if !errors.Is(err, pgx.ErrNoRows) { + return engine.ResolvedIdentity{}, err + } + // Not linked to THIS installation. Before prompting, reuse a link the same + // Slack user already made to another installation of the same team in this + // workspace (MUL-3911): one link per Slack workspace, not per app. + cand, ok, ferr := r.reusableBinding(ctx, inst, senderID) + if ferr != nil { + return engine.ResolvedIdentity{}, ferr + } + if !ok { return engine.ResolvedIdentity{}, engine.ErrSenderUnbound } - return engine.ResolvedIdentity{}, err + binding, reused = cand, true } - // Binding existence no longer proves membership (no FK); re-check. + // Binding existence no longer proves membership (no FK); re-check. For a + // reused link this also gates materialization: we never persist a binding for + // a user who has since left the workspace. if _, err := r.q.GetMemberByUserAndWorkspace(ctx, db.GetMemberByUserAndWorkspaceParams{ UserID: binding.MulticaUserID, WorkspaceID: inst.WorkspaceID, }); err != nil { if errors.Is(err, pgx.ErrNoRows) { + if reused { + // Same human, no longer a member: prompt a fresh link rather than + // surface "not a member" for an app they never linked. + return engine.ResolvedIdentity{}, engine.ErrSenderUnbound + } return engine.ResolvedIdentity{}, engine.ErrSenderNotMember } return engine.ResolvedIdentity{}, err } + if reused { + // Materialize the reused link as a binding on THIS installation so later + // messages resolve on the fast per-installation path and are pruned with + // the member like any other. Idempotent via ON CONFLICT; a concurrent + // first message that already wrote it returns the same row. + if _, err := r.q.CreateChannelUserBinding(ctx, db.CreateChannelUserBindingParams{ + WorkspaceID: inst.WorkspaceID, + MulticaUserID: binding.MulticaUserID, + InstallationID: inst.ID, + ChannelType: string(TypeSlack), + ChannelUserID: senderID, + Config: []byte(`{}`), + }); err != nil { + return engine.ResolvedIdentity{}, fmt.Errorf("materialize reused slack binding: %w", err) + } + } return engine.ResolvedIdentity{UserID: binding.MulticaUserID}, nil } +// reusableBinding looks for a link the same Slack user already made to ANOTHER +// installation of the SAME workspace + SAME Slack team, so a second app in one +// Slack workspace need not re-prompt (MUL-3911). ok=false (nil error) means "no +// reuse — prompt to link": the installation records no team (legacy), its +// Platform is not a ChannelInstallation, or no matching binding exists. +func (r *identityResolver) reusableBinding(ctx context.Context, inst engine.ResolvedInstallation, senderID string) (db.ChannelUserBinding, bool, error) { + ci, ok := inst.Platform.(db.ChannelInstallation) + if !ok { + return db.ChannelUserBinding{}, false, nil + } + teamID := installTeamID(ci.Config) + if teamID == "" { + return db.ChannelUserBinding{}, false, nil + } + cand, err := r.q.FindReusableChannelUserBinding(ctx, db.FindReusableChannelUserBindingParams{ + WorkspaceID: inst.WorkspaceID, + ChannelType: string(TypeSlack), + ChannelUserID: senderID, + TeamID: teamID, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return db.ChannelUserBinding{}, false, nil + } + return db.ChannelUserBinding{}, false, err + } + return cand, true, nil +} + // ---- dedup ---- type deduper struct{ q *db.Queries } diff --git a/server/pkg/db/generated/channel.sql.go b/server/pkg/db/generated/channel.sql.go index 564111293..0f770a41f 100644 --- a/server/pkg/db/generated/channel.sql.go +++ b/server/pkg/db/generated/channel.sql.go @@ -402,6 +402,59 @@ func (q *Queries) DeleteChannelUserBindingsByWorkspaceMember(ctx context.Context return err } +const findReusableChannelUserBinding = `-- name: FindReusableChannelUserBinding :one +SELECT b.id, b.workspace_id, b.multica_user_id, b.installation_id, b.channel_type, b.channel_user_id, b.config, b.bound_at FROM channel_user_binding b +JOIN channel_installation ci ON ci.id = b.installation_id +WHERE b.workspace_id = $1 + AND b.channel_type = $2 + AND b.channel_user_id = $3 + AND ci.config ->> 'team_id' = $4::text +ORDER BY b.bound_at DESC +LIMIT 1 +` + +type FindReusableChannelUserBindingParams struct { + WorkspaceID pgtype.UUID `json:"workspace_id"` + ChannelType string `json:"channel_type"` + ChannelUserID string `json:"channel_user_id"` + TeamID string `json:"team_id"` +} + +// Cross-installation account-link reuse (MUL-3911). When a platform user +// messages an installation they have NOT linked, but the SAME user id is already +// bound to ANOTHER installation in the SAME Multica workspace + SAME Slack team, +// the inbound identity step reuses that link instead of re-prompting. Slack user +// ids are stable within a team, so an identical channel_user_id denotes the same +// human across that team's apps. The match is fenced to one workspace AND one +// team (installation config->>'team_id'): a Slack team can be connected to two +// different Multica workspaces, and a user may hold different Multica accounts in +// each, so reuse must cross neither boundary. Most-recently-bound wins. The +// caller re-checks membership and materializes a fresh per-installation binding. +// +// team_id is pinned ::text so sqlc types the arg as a string instead of +// attributing the bare param to the JSONB config column (mirrors +// GetChannelInstallationByAppID's app_id cast). +func (q *Queries) FindReusableChannelUserBinding(ctx context.Context, arg FindReusableChannelUserBindingParams) (ChannelUserBinding, error) { + row := q.db.QueryRow(ctx, findReusableChannelUserBinding, + arg.WorkspaceID, + arg.ChannelType, + arg.ChannelUserID, + arg.TeamID, + ) + var i ChannelUserBinding + err := row.Scan( + &i.ID, + &i.WorkspaceID, + &i.MulticaUserID, + &i.InstallationID, + &i.ChannelType, + &i.ChannelUserID, + &i.Config, + &i.BoundAt, + ) + return i, err +} + const getChannelChatSessionBinding = `-- name: GetChannelChatSessionBinding :one SELECT id, chat_session_id, installation_id, channel_type, channel_chat_id, chat_type, last_message_id, last_thread_id, config, created_at FROM channel_chat_session_binding WHERE installation_id = $1 AND channel_chat_id = $2 diff --git a/server/pkg/db/queries/channel.sql b/server/pkg/db/queries/channel.sql index 1e76f5197..71de3af19 100644 --- a/server/pkg/db/queries/channel.sql +++ b/server/pkg/db/queries/channel.sql @@ -227,6 +227,30 @@ RETURNING *; SELECT * FROM channel_user_binding WHERE installation_id = $1 AND channel_user_id = $2; +-- name: FindReusableChannelUserBinding :one +-- Cross-installation account-link reuse (MUL-3911). When a platform user +-- messages an installation they have NOT linked, but the SAME user id is already +-- bound to ANOTHER installation in the SAME Multica workspace + SAME Slack team, +-- the inbound identity step reuses that link instead of re-prompting. Slack user +-- ids are stable within a team, so an identical channel_user_id denotes the same +-- human across that team's apps. The match is fenced to one workspace AND one +-- team (installation config->>'team_id'): a Slack team can be connected to two +-- different Multica workspaces, and a user may hold different Multica accounts in +-- each, so reuse must cross neither boundary. Most-recently-bound wins. The +-- caller re-checks membership and materializes a fresh per-installation binding. +-- +-- team_id is pinned ::text so sqlc types the arg as a string instead of +-- attributing the bare param to the JSONB config column (mirrors +-- GetChannelInstallationByAppID's app_id cast). +SELECT b.* FROM channel_user_binding b +JOIN channel_installation ci ON ci.id = b.installation_id +WHERE b.workspace_id = sqlc.arg('workspace_id') + AND b.channel_type = sqlc.arg('channel_type') + AND b.channel_user_id = sqlc.arg('channel_user_id') + AND ci.config ->> 'team_id' = sqlc.arg('team_id')::text +ORDER BY b.bound_at DESC +LIMIT 1; + -- name: DeleteChannelUserBindingsByWorkspaceMember :exec -- Application-layer integrity (replaces the old member-FK ON DELETE -- CASCADE): prune every binding for a user who has been removed from a