Remove redundant channel auth, add per-user notify preference#70
Remove redundant channel auth, add per-user notify preference#70
Conversation
…nels Access control and notification routing are now fully handled by the RBAC system (auth_identities + policy engine). Remove the redundant per-channel AllowedIDs allowlist and NotifyChat/defaultChat static fallback from all channels (telegram, qq, feishu, weixin), the dispatcher, gateway wiring, admin UI, and docs.
Add notify_identity_id FK on auth_users pointing to auth_identities. NotifyUser now sends to a single channel instead of all linked ones: - If the user set a preferred identity, use that. - Otherwise use the first linked identity (earliest linked_at). - ON DELETE SET NULL cascades when an identity is unlinked. Includes admin UI dropdown, API endpoint, migration, and tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20be4b3569
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return func(_ *dto.WSPayload, data *dto.WSC2CMessageData) error { | ||
| msg := (*dto.Message)(data) | ||
| authorID := msg.Author.ID | ||
| if !b.isAllowed(authorID) { | ||
| logger().Warn("unauthorized c2c access", "user_id", authorID) | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
Reinstate sender gating before resolving unlinked users
Removing the channel-level isAllowed check here (and in the analogous Telegram/Feishu/Weixin handlers) opens the bot to any external sender, because the RBAC path does not stop unlinked accounts. ResolveUser explicitly returns a zero-value user when no identity is linked (internal/channel/identity.go:30-40), and ResolveAgent treats that as hasAuth == false, so canAccess returns true for any enabled agent (internal/channel/identity.go:66-82). Deployments that previously relied on allowed_ids to keep the bot private will now accept arbitrary inbound messages and run agent tools for them.
Useful? React with 👍 / 👎.
| func (b *Bot) Notify(ctx context.Context, n channel.Notification) error { | ||
| chatID := n.ChatID | ||
| if chatID == "" { | ||
| chatID = b.cfg.NotifyChat | ||
| } | ||
| if chatID == "" { | ||
| return fmt.Errorf("feishu: no target chat ID") |
There was a problem hiding this comment.
Keep a default target for system notification broadcasts
dispatcher.Notify() is still used for system-owned scheduler jobs and heartbeat results with an empty ChatID (cmd/anna/gateway.go:291-299, internal/scheduler/heartbeat.go:104-107, and internal/channel/notify_tool.go:61-66), but this change makes Feishu fail immediately whenever n.ChatID is blank. Since the same commit also removes notify_chat/registered defaults, existing installs that used Feishu for proactive notifications will stop receiving heartbeat, scheduler, and notify broadcasts unless every caller starts passing an explicit chat ID. The same regression exists in weixin.Notify, and Telegram also regresses when only notify_chat was configured.
Useful? React with 👍 / 👎.
Summary
Remove
NotifyChat,AllowedIDs, anddefaultChatfrom all channels (telegram, qq, feishu, weixin), dispatcher, gateway, admin UI, and docs. Access control is now fully handled by RBAC (auth_identities+ policy engine), and notification targets are resolved from linked identities.Add per-user notify channel preference via
notify_identity_idFK onauth_users→auth_identities.NotifyUsernow sends to a single channel (preferred identity or first linked) instead of broadcasting to all linked channels.ON DELETE SET NULLauto-clears preference when an identity is unlinked.Net: -529 lines, 22+16 files changed across 2 commits.
Changes
Commit 1: Remove redundant fields
AllowedIDsconfig +isAllowed()from all 4 channelsNotifyChatconfig + fallback from all Notify() methodsdefaultChatfromDispatcher.Register()Commit 2: Per-user notify preference
notify_identity_id INTEGER REFERENCES auth_identities(id) ON DELETE SET NULLNotifyIdentityID *int64onAuthUser,UpdateUserNotifyIdentitystore methodNotifyUserpicks single channel (preferred > first linked)PUT /api/users/{id}/notify-identityTest plan
mise run lint— 0 issuesmise run test— all channel/auth/db tests pass (429 tests)