diff --git a/web/app/(dashboard)/dashboard/channels/page.tsx b/web/app/(dashboard)/dashboard/channels/page.tsx
index aa8c06f7..990bfc8b 100644
--- a/web/app/(dashboard)/dashboard/channels/page.tsx
+++ b/web/app/(dashboard)/dashboard/channels/page.tsx
@@ -5,6 +5,7 @@ import {
MessagesSquareIcon,
MessageSquareMoreIcon,
SendIcon,
+ SlackIcon,
} from "lucide-react"
import {
@@ -39,6 +40,9 @@ function getChannelTypeLabel(channelType: string, t: (key: string) => string) {
if (channelType === "zalo_oa") {
return t("channel.typeZaloOa")
}
+ if (channelType === "slack") {
+ return t("channel.typeSlack")
+ }
return t("channel.typeWeb")
}
@@ -62,6 +66,9 @@ function ChannelIcon({ channelType }: { channelType: string }) {
if (channelType === "telegram" || channelType === "zalo_oa") {
return
}
+ if (channelType === "slack") {
+ return
+ }
return
}
@@ -78,6 +85,7 @@ export default function DashboardChannelsPage() {
{ value: "all", label: t("channel.allTypes") },
{ value: "web", label: t("channel.typeWeb") },
{ value: "telegram", label: t("channel.typeTelegram") },
+ { value: "slack", label: t("channel.typeSlack") },
{ value: "zalo_oa", label: t("channel.typeZaloOa") },
{ value: "wechat_mp", label: t("channel.typeWechatMp") },
{ value: "wxwork_kf", label: t("channel.typeWxworkKf") },
diff --git a/web/messages/en-US.json b/web/messages/en-US.json
index c83bc000..06cb4a51 100644
--- a/web/messages/en-US.json
+++ b/web/messages/en-US.json
@@ -624,6 +624,18 @@
"zaloAppId": "App ID",
"zaloAutoConnectTitle": "Zalo Official Account Connection",
"zaloAutoConnectDescription": "Connect your Zalo Official Account using the Access Token to automatically receive customer messages and dispatch replies.",
+ "typeSlack": "Slack",
+ "slackBotToken": "Bot User OAuth Token",
+ "slackBotTokenRequired": "Slack Bot User OAuth Token is required",
+ "slackSigningSecret": "Signing Secret",
+ "slackSigningSecretHint": "Required to verify inbound events. Without it the webhook accepts unsigned requests, so anyone who learns the URL can post into a conversation.",
+ "slackAppId": "App ID",
+ "slackTeamId": "Workspace ID",
+ "slackTeamName": "Workspace Name",
+ "slackDefaultChannel": "Default Channel ID",
+ "slackSetupTitle": "Slack App Connection",
+ "slackSetupDescription": "Create a Slack app, install it to the workspace, and copy the Bot User OAuth Token and Signing Secret from Basic Information. Subscribe to message.events, then set the Request URL below in Event Subscriptions.",
+ "slackRequestUrl": "Request URL",
"loadFailed": "Could not load channels.",
"created": "Channel created: {name}",
"updated": "Channel updated: {name}",
diff --git a/web/messages/zh-CN.json b/web/messages/zh-CN.json
index 14e67ae3..f73f838f 100644
--- a/web/messages/zh-CN.json
+++ b/web/messages/zh-CN.json
@@ -624,6 +624,18 @@
"zaloAppId": "App ID",
"zaloAutoConnectTitle": "Zalo OA 渠道连接",
"zaloAutoConnectDescription": "输入 Zalo OA 的 Access Token 即可自动双向同步客户会话与消息。",
+ "typeSlack": "Slack",
+ "slackBotToken": "Bot User OAuth Token",
+ "slackBotTokenRequired": "请填写 Slack Bot User OAuth Token",
+ "slackSigningSecret": "Signing Secret",
+ "slackSigningSecretHint": "用于校验入站事件，建议填写。留空时该 Webhook 会接受未签名请求，任何知道地址的人都能向会话发消息。",
+ "slackAppId": "App ID",
+ "slackTeamId": "工作区 ID",
+ "slackTeamName": "工作区名称",
+ "slackDefaultChannel": "默认频道 ID",
+ "slackSetupTitle": "Slack 应用接入",
+ "slackSetupDescription": "创建 Slack 应用并安装到工作区，在 Basic Information 页面复制 Bot User OAuth Token 和 Signing Secret。订阅 message.events 事件，再把下方的 Request URL 填入 Event Subscriptions。",
+ "slackRequestUrl": "Request URL",
"loadFailed": "加载接入渠道失败",
"created": "已创建接入渠道：{name}",
"updated": "已更新接入渠道：{name}",
From cfdc56321db30d0fc4e812cd16b559406b3e011d Mon Sep 17 00:00:00 2001
From: JOY <5027251+JOY@users.noreply.github.com>
Date: Tue, 22 Sep 2026 09:44:48 +0700
Subject: [PATCH 4/8] fix(auth): stop granting admin to every first-time OIDC
login
ensureDefaultOIDCRole assigned the admin role (falling back to
super_admin) to any user that arrived without a role, which in practice
means every first-time OIDC login: a stranger with an account at the
identity provider became an administrator of the support desk. Give the
lowest staff role (cs_user) instead; administrative access must be
granted deliberately.
The regression test seeds all three candidate roles and asserts a
first-time OIDC user lands on cs_user and nothing else.
---
internal/services/oidc_login_service.go | 9 ++--
internal/services/oidc_login_service_test.go | 57 ++++++++++++++++++++
2 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/internal/services/oidc_login_service.go b/internal/services/oidc_login_service.go
index 9827c4d7..e764500f 100644
--- a/internal/services/oidc_login_service.go
+++ b/internal/services/oidc_login_service.go
@@ -257,6 +257,10 @@ func shortSubjectHash(subject string) string {
return hex.EncodeToString(sum[:])[:16]
}
+// ensureDefaultOIDCRole gives a first-time OIDC user the lowest staff role.
+// The provider vouches for who the user is, not what they may administer, so
+// a user arriving without any role must never escalate to an administrative
+// one.
func (s *oidcLoginService) ensureDefaultOIDCRole(tx *gorm.DB, user *models.User) {
if user == nil || user.ID <= 0 {
return
@@ -265,10 +269,7 @@ func (s *oidcLoginService) ensureDefaultOIDCRole(tx *gorm.DB, user *models.User)
if existingRole != nil {
return
}
- defaultRole := repositories.RoleRepository.GetByCode(tx, constants.RoleCodeAdmin)
- if defaultRole == nil {
- defaultRole = repositories.RoleRepository.GetByCode(tx, constants.RoleCodeSuperAdmin)
- }
+ defaultRole := repositories.RoleRepository.GetByCode(tx, constants.RoleCodeCsUser)
if defaultRole == nil {
return
}
diff --git a/internal/services/oidc_login_service_test.go b/internal/services/oidc_login_service_test.go
index 4e0749d7..daa1909e 100644
--- a/internal/services/oidc_login_service_test.go
+++ b/internal/services/oidc_login_service_test.go
@@ -3,9 +3,11 @@ package services
import (
"strings"
"testing"
+ "time"
"agent-desk/internal/models"
"agent-desk/internal/pkg/config"
+ "agent-desk/internal/pkg/constants"
"agent-desk/internal/pkg/enums"
)
@@ -94,3 +96,58 @@ func TestOIDCLoginReusesExistingIdentity(t *testing.T) {
t.Fatalf("expected existing identity to reuse user, got %d users", count)
}
}
+
+// A first-time OIDC user is a stranger the provider vouched for, nothing
+// more: they must land on the lowest staff role, never on an administrative
+// one. Seeds every candidate role so the assignment cannot pass by accident
+// of a missing row.
+func TestOIDCLoginFirstUserGetsLowestStaffRole(t *testing.T) {
+ db := setupAuthServiceTestDB(t)
+ now := time.Now()
+ for _, role := range []struct{ name, code string }{
+ {"Super Admin", constants.RoleCodeSuperAdmin},
+ {"Admin", constants.RoleCodeAdmin},
+ {"Support Agent", constants.RoleCodeCsUser},
+ } {
+ if err := db.Create(&models.Role{
+ Name: role.name,
+ Code: role.code,
+ Status: enums.StatusOk,
+ AuditFields: models.AuditFields{
+ CreatedAt: now,
+ UpdatedAt: now,
+ },
+ }).Error; err != nil {
+ t.Fatalf("seed role %s: %v", role.code, err)
+ }
+ }
+
+ if _, err := newOIDCLoginService().loginWithOIDCProfile(&oidcLoginProfile{
+ Subject: "sub-777",
+ Email: "stranger@example.com",
+ PreferredUsername: "stranger",
+ Name: "Stranger",
+ RawProfile: `{"sub":"sub-777"}`,
+ }, config.AuthConfig{TokenTTLHours: 2}, "127.0.0.1", "go-test"); err != nil {
+ t.Fatalf("loginWithOIDCProfile() error = %v", err)
+ }
+
+ var user models.User
+ if err := db.Take(&user, "username = ?", "stranger").Error; err != nil {
+ t.Fatalf("expected OIDC user to be created: %v", err)
+ }
+
+ var roles []models.Role
+ if err := db.
+ Joins("JOIN t_user_role ON t_user_role.role_id = t_role.id").
+ Where("t_user_role.user_id = ?", user.ID).
+ Find(&roles).Error; err != nil {
+ t.Fatalf("query user roles: %v", err)
+ }
+ if len(roles) != 1 {
+ t.Fatalf("expected exactly one role for a first-time OIDC user, got %d", len(roles))
+ }
+ if roles[0].Code != constants.RoleCodeCsUser {
+ t.Fatalf("first-time OIDC user role = %q, want %q", roles[0].Code, constants.RoleCodeCsUser)
+ }
+}
From fd82111fa3c76395fb8154d34199199d98f4a372 Mon Sep 17 00:00:00 2001
From: JOY <5027251+JOY@users.noreply.github.com>
Date: Sun, 13 Sep 2026 16:35:07 +0700
Subject: [PATCH 5/8] fix(security): make the client IP trustworthy and scope
credential lockout to it
Two coupled defects. The second could not be fixed safely before the first.
Gin was never told which proxies to trust. Nothing in the repository calls
SetTrustedProxies or sets TrustedPlatform, so the engine ran on Gin's own default
of 0.0.0.0/0 and ::/0 - every peer is a trusted proxy. validateHeader then walks
X-Forwarded-For right to left and stops at the first address that is *not* a
trusted proxy; with everything trusted it never stops, and returns the leftmost
value, which is the one the caller supplied. Any client could therefore choose the
address recorded in t_login_credential_log.client_ip and t_user.last_login_ip by
sending one header. X-Real-IP is in Gin's default RemoteIPHeaders too, so it was a
second route to the same result.
- ServerConfig gains trustedProxies and trustedPlatform. Left unset,
trustedProxies falls back to the loopback, RFC1918, IPv6 unique-local and
link-local ranges rather than to Gin's trust-everything default. That covers a
sidecar tunnel, a compose network and a local nginx, and a client on the public
internet cannot present one of those addresses as its direct peer.
- trustedPlatform maps "cloudflare", "fly.io" and "google-app-engine" onto Gin's
own constants and passes any other value through as a literal header name. It
defaults to empty on purpose: an edge that appends rather than overwrites would
turn the header back into a client-controlled value.
- NewServer applies both before any middleware is registered, and fails startup on
an unparseable CIDR rather than quietly dropping the trust boundary.
- Documented in config/config.example.yaml, and .env.example sets
TRUSTED_PLATFORM=cloudflare because this deployment sits behind a Cloudflare
Tunnel.
With an address that can be believed, credential lockout stops being a denial of
service. isCredentialLocked keyed on the username alone, so anybody who knew a
username could lock the real account out for the whole window, from anywhere, as
often as they liked - and the window was renewable indefinitely.
- The per-account window is now keyed on principal *and* client address, so one
source grinding on one account is still stopped.
- A second window counts failures from one address across all principals, which is
what credential stuffing looks like. auth.maxFailedAttemptsPerIP configures it
and defaults to four times maxFailedAttempts; disabling maxFailedAttempts
disables both, matching the existing convention.
- An address the server could not determine is normalised to a single "unknown"
bucket and excluded from the per-address window, so a missing IP cannot pool
every such caller into one lockout.
- createLoginCredentialLog normalises on write so the read and write sides cannot
drift apart.
Tests. TestGinDefaultTrustsEveryProxy pins the vulnerable default, so the fix
stays load-bearing and a future Gin that changes the default says so instead of
silently making the configuration decorative. Three more cover an untrusted peer's
forged header being ignored, a trusted proxy's appended address beating
client-prepended junk, and the platform header taking precedence over
X-Forwarded-For. Two lockout tests cover the denial of service being closed and
the per-address window catching a username spray that no single account trips.
Four existing lockout tests seeded credential logs without a client address, which
is exactly the assumption this removes, so they now seed one.
---
.env.example | 11 ++
config/config.example.yaml | 12 ++
internal/bootstrap/server.go | 13 ++
internal/bootstrap/server_clientip_test.go | 148 +++++++++++++++++++++
internal/pkg/config/config.go | 84 +++++++++++-
internal/pkg/config/config_test.go | 104 +++++++++++++++
internal/services/auth_service.go | 67 ++++++++--
internal/services/auth_service_test.go | 79 +++++++++++
8 files changed, 505 insertions(+), 13 deletions(-)
create mode 100644 internal/bootstrap/server_clientip_test.go
diff --git a/.env.example b/.env.example
index 12afb717..edbe2b24 100644
--- a/.env.example
+++ b/.env.example
@@ -9,6 +9,17 @@ COMPANY_NAME=
COMPANY_LOGO_URL=
# AGENT_DESK_SERVER_CORS_ALLOWEDORIGINS="http://localhost:3000,http://127.0.0.1:8083"
+# Client IP resolution.
+# Gin defaults to trusting every proxy, which makes X-Forwarded-For authoritative:
+# any caller could then choose the IP recorded in the login credential log and in
+# user.last_login_ip, and any IP-keyed rate limit or lockout would be bypassable.
+# Unset, trustedProxies falls back to the loopback / RFC1918 / IPv6 unique-local /
+# link-local ranges, which already covers a cloudflared sidecar on a compose
+# network. Set trustedPlatform when an edge overwrites the header instead of
+# appending to it - Cloudflare does, and CF-Connecting-IP is not client-forgeable.
+# TRUSTED_PROXIES="10.0.0.0/8,172.16.0.0/12"
+TRUSTED_PLATFORM=cloudflare
+
# Database Configuration
# Driver options: sqlite, mysql, postgres
DB_TYPE=sqlite
diff --git a/config/config.example.yaml b/config/config.example.yaml
index 4dd9f45f..d719f25d 100644
--- a/config/config.example.yaml
+++ b/config/config.example.yaml
@@ -5,6 +5,18 @@ server:
# Custom platform / instance branding (optional)
companyName: ""
companyLogoUrl: ""
+ # Reverse proxies in front of the application, as CIDR blocks. Gin's own default
+ # trusts every peer, which makes X-Forwarded-For authoritative and lets any
+ # caller choose the client address the server records in the login credential
+ # log and in user.last_login_ip. Left empty this falls back to the loopback,
+ # RFC1918, IPv6 unique-local and link-local ranges, which covers a sidecar
+ # tunnel, a compose network and a local nginx. Set it explicitly when the proxy
+ # sits on a public address, and never set it to 0.0.0.0/0.
+ trustedProxies: []
+ # Set to "cloudflare" (also accepted: "fly.io", "google-app-engine", or a literal
+ # header name) when an edge overwrites rather than appends the real client
+ # address. When set it takes precedence over X-Forwarded-For entirely.
+ trustedPlatform: ""
cors:
# Browser CORS allowlist. In production, replace this with the actual frontend or embedded-site domains, such as https://support.example.com.
# Leave it empty to reject cross-origin browser requests. Same-origin and non-browser calls are still supported.
diff --git a/internal/bootstrap/server.go b/internal/bootstrap/server.go
index 93dcaf50..16416620 100644
--- a/internal/bootstrap/server.go
+++ b/internal/bootstrap/server.go
@@ -1,6 +1,7 @@
package bootstrap
import (
+ "fmt"
"log/slog"
"net/http"
"path"
@@ -35,6 +36,18 @@ func NewServer() (*gin.Engine, error) {
printBanner()
app := gin.New()
+
+ // Gin defaults to trusting every proxy, which makes ClientIP() return the
+ // leftmost X-Forwarded-For value - a header any caller can set. Everything
+ // keyed on a client address depends on this being settled first: the login
+ // credential log, the user's last login IP, and any abuse control.
+ if platform := cfg.Server.TrustedPlatformHeader(); platform != "" {
+ app.TrustedPlatform = platform
+ }
+ if err := app.SetTrustedProxies(cfg.Server.TrustedProxiesOrDefault()); err != nil {
+ return nil, fmt.Errorf("invalid server.trustedProxies: %w", err)
+ }
+
app.Use(requestIDMiddleware())
app.Use(corsMiddleware())
app.Use(gin.Recovery())
diff --git a/internal/bootstrap/server_clientip_test.go b/internal/bootstrap/server_clientip_test.go
new file mode 100644
index 00000000..2711eb24
--- /dev/null
+++ b/internal/bootstrap/server_clientip_test.go
@@ -0,0 +1,148 @@
+package bootstrap
+
+import (
+ "bytes"
+ "log/slog"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "agent-desk/internal/pkg/config"
+
+ "github.com/gin-gonic/gin"
+)
+
+// captureRequestLog swaps the slog default for one writing into buf, because
+// requestLogMiddleware records the address Gin resolved. That is the only place
+// the server exposes its ClientIP() decision, and it is the value that ends up in
+// t_login_credential_log and t_user.last_login_ip.
+func captureRequestLog(t *testing.T) *bytes.Buffer {
+ t.Helper()
+ var buf bytes.Buffer
+ previous := slog.Default()
+ slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil)))
+ t.Cleanup(func() { slog.SetDefault(previous) })
+ return &buf
+}
+
+func setTestServerConfig(server config.ServerConfig) {
+ config.SetCurrent(&config.Config{
+ Server: server,
+ Storage: config.StorageConfig{Local: config.LocalStorageConfig{Root: "storage", BaseURL: "/storage"}},
+ })
+}
+
+// TestGinDefaultTrustsEveryProxy pins the vulnerability the configuration above
+// exists to close. A bare Gin engine - which is what NewServer used to build -
+// trusts 0.0.0.0/0 and ::/0, so validateHeader walks X-Forwarded-For right to
+// left, finds no untrusted proxy to stop at, and returns the leftmost value the
+// caller chose. If this assertion ever starts failing, Gin's default has changed
+// and the trusted-proxy configuration should be revisited rather than assumed
+// necessary.
+func TestGinDefaultTrustsEveryProxy(t *testing.T) {
+ app := gin.New()
+ var resolved string
+ app.GET("/probe", func(ctx *gin.Context) { resolved = ctx.ClientIP() })
+
+ req := httptest.NewRequest(http.MethodGet, "/probe", nil)
+ req.RemoteAddr = "203.0.113.7:52000"
+ req.Header.Set("X-Forwarded-For", "198.51.100.9")
+ app.ServeHTTP(httptest.NewRecorder(), req)
+
+ if resolved != "198.51.100.9" {
+ t.Fatalf("a bare engine resolved ClientIP to %q, expected the forged 198.51.100.9", resolved)
+ }
+}
+
+func TestNewServerIgnoresForgedForwardedForFromAnUntrustedPeer(t *testing.T) {
+ buf := captureRequestLog(t)
+ setTestServerConfig(config.ServerConfig{})
+
+ app, err := NewServer()
+ if err != nil {
+ t.Fatalf("NewServer() error = %v", err)
+ }
+
+ rec := httptest.NewRecorder()
+ req := httptest.NewRequest(http.MethodGet, "/api/health", nil)
+ // A public peer is outside every default trusted range, so nothing it claims
+ // about the originating address may be believed.
+ req.RemoteAddr = "203.0.113.7:52000"
+ req.Header.Set("X-Forwarded-For", "198.51.100.9")
+ req.Header.Set("X-Real-IP", "198.51.100.9")
+ app.ServeHTTP(rec, req)
+
+ logged := buf.String()
+ if !strings.Contains(logged, "clientIp=203.0.113.7") {
+ t.Fatalf("expected the real peer address to be logged, got: %s", logged)
+ }
+ if strings.Contains(logged, "198.51.100.9") {
+ t.Fatalf("a forged forwarding header was trusted: %s", logged)
+ }
+}
+
+func TestNewServerReadsForwardedForFromATrustedProxy(t *testing.T) {
+ buf := captureRequestLog(t)
+ setTestServerConfig(config.ServerConfig{TrustedProxies: []string{"10.0.0.0/8"}})
+
+ app, err := NewServer()
+ if err != nil {
+ t.Fatalf("NewServer() error = %v", err)
+ }
+
+ rec := httptest.NewRecorder()
+ req := httptest.NewRequest(http.MethodGet, "/api/health", nil)
+ req.RemoteAddr = "10.0.0.5:52000"
+ // A hostile client prepends a value it chose; the proxy appends the address it
+ // actually saw. Gin walks the list right to left and stops at the first
+ // address that is not itself a trusted proxy, which is the appended one.
+ req.Header.Set("X-Forwarded-For", "198.51.100.9, 203.0.113.7")
+ app.ServeHTTP(rec, req)
+
+ logged := buf.String()
+ if !strings.Contains(logged, "clientIp=203.0.113.7") {
+ t.Fatalf("expected the proxy-appended address, got: %s", logged)
+ }
+ if strings.Contains(logged, "198.51.100.9") {
+ t.Fatalf("the client-prepended address was trusted: %s", logged)
+ }
+}
+
+func TestNewServerPrefersTheConfiguredTrustedPlatformHeader(t *testing.T) {
+ buf := captureRequestLog(t)
+ setTestServerConfig(config.ServerConfig{TrustedPlatform: "cloudflare"})
+
+ app, err := NewServer()
+ if err != nil {
+ t.Fatalf("NewServer() error = %v", err)
+ }
+
+ rec := httptest.NewRecorder()
+ req := httptest.NewRequest(http.MethodGet, "/api/health", nil)
+ req.RemoteAddr = "10.0.0.5:52000"
+ req.Header.Set("X-Forwarded-For", "198.51.100.9")
+ // The edge overwrites this header rather than appending to it, which is what
+ // makes it usable as an identity at all.
+ req.Header.Set("CF-Connecting-IP", "203.0.113.7")
+ app.ServeHTTP(rec, req)
+
+ logged := buf.String()
+ if !strings.Contains(logged, "clientIp=203.0.113.7") {
+ t.Fatalf("expected the platform header to win, got: %s", logged)
+ }
+ if strings.Contains(logged, "198.51.100.9") {
+ t.Fatalf("X-Forwarded-For was consulted despite a trusted platform: %s", logged)
+ }
+}
+
+func TestNewServerRejectsAnInvalidTrustedProxyCIDR(t *testing.T) {
+ captureRequestLog(t)
+ setTestServerConfig(config.ServerConfig{TrustedProxies: []string{"10.0.0.0/8", "not-a-cidr"}})
+
+ if _, err := NewServer(); err == nil {
+ t.Fatal("NewServer() accepted an unparseable CIDR; a typo here would silently disable the trust boundary")
+ } else if !strings.Contains(err.Error(), "server.trustedProxies") {
+ t.Fatalf("NewServer() error = %v, expected it to name server.trustedProxies", err)
+ }
+}
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index 5925d94d..8d224a64 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -52,6 +52,65 @@ type ServerConfig struct {
CompanyName string `yaml:"companyName"`
CompanyLogoURL string `yaml:"companyLogoUrl"`
CORS CORSConfig `yaml:"cors"`
+ // TrustedProxies are the CIDR blocks of the reverse proxies that sit in front
+ // of the application. Gin's own default is 0.0.0.0/0 and ::/0, which trusts
+ // every peer and makes ClientIP() return the leftmost X-Forwarded-For value -
+ // a header any caller can set.
+ TrustedProxies []string `yaml:"trustedProxies"`
+ // TrustedPlatform names an edge that overwrites rather than appends the real
+ // client address, for example "cloudflare". When set it takes precedence over
+ // X-Forwarded-For entirely.
+ TrustedPlatform string `yaml:"trustedPlatform"`
+}
+
+// defaultTrustedProxies covers loopback, RFC1918, IPv6 unique-local and
+// link-local ranges. That is the shape of almost every real deployment - a
+// sidecar tunnel, a compose network, a local nginx - and a client on the public
+// internet cannot present one of these addresses as its direct peer, so
+// X-Forwarded-For stays honest. When the app is exposed directly the peer is a
+// public address, is not trusted, and Gin falls back to it.
+var defaultTrustedProxies = []string{
+ "127.0.0.0/8",
+ "10.0.0.0/8",
+ "172.16.0.0/12",
+ "192.168.0.0/16",
+ "::1/128",
+ "fc00::/7",
+ "fe80::/10",
+}
+
+func (s ServerConfig) TrustedProxiesOrDefault() []string {
+ proxies := make([]string, 0, len(s.TrustedProxies))
+ for _, proxy := range s.TrustedProxies {
+ if proxy = strings.TrimSpace(proxy); proxy != "" {
+ proxies = append(proxies, proxy)
+ }
+ }
+ if len(proxies) == 0 {
+ return defaultTrustedProxies
+ }
+ return proxies
+}
+
+// TrustedPlatformHeader resolves the configured platform name to the header Gin
+// should read the client address from. Recognised names map to Gin's own
+// constants; any other non-empty value is passed through as a literal header
+// name, which is what Gin's TrustedPlatform field expects.
+func (s ServerConfig) TrustedPlatformHeader() string {
+ platform := strings.TrimSpace(s.TrustedPlatform)
+ if platform == "" {
+ return ""
+ }
+ switch strings.ToLower(platform) {
+ case "cloudflare", "cf":
+ return "CF-Connecting-IP"
+ case "fly.io", "flyio", "fly-io":
+ return "Fly-Client-IP"
+ case "google-app-engine", "appengine", "gae":
+ return "X-Appengine-Remote-Addr"
+ default:
+ return platform
+ }
}
func (s ServerConfig) Address() string {
@@ -86,7 +145,25 @@ type AuthConfig struct {
PasswordLoginEnabled *bool `yaml:"passwordLoginEnabled"`
TokenTTLHours int `yaml:"tokenTTLHours"`
MaxFailedAttempts int `yaml:"maxFailedAttempts"`
- CredentialLockMinute int `yaml:"credentialLockMinute"`
+ // MaxFailedAttemptsPerIP bounds failures from one client address across every
+ // username, which is what credential stuffing looks like. Zero or unset
+ // derives four times MaxFailedAttempts; it is disabled when MaxFailedAttempts
+ // is disabled.
+ MaxFailedAttemptsPerIP int `yaml:"maxFailedAttemptsPerIP"`
+ CredentialLockMinute int `yaml:"credentialLockMinute"`
+}
+
+// MaxFailedAttemptsPerIPOrDefault derives the per-address threshold from the
+// per-account one so that a deployment which only tunes MaxFailedAttempts still
+// gets a coherent pair of limits.
+func (a AuthConfig) MaxFailedAttemptsPerIPOrDefault() int {
+ if a.MaxFailedAttemptsPerIP > 0 {
+ return a.MaxFailedAttemptsPerIP
+ }
+ if a.MaxFailedAttempts <= 0 {
+ return 0
+ }
+ return a.MaxFailedAttempts * 4
}
func (a AuthConfig) IsPasswordLoginEnabled() bool {
@@ -301,6 +378,8 @@ func bindConfigDefaults(v *viper.Viper) {
v.SetDefault("server.companyName", "")
v.SetDefault("server.companyLogoUrl", "")
v.SetDefault("server.cors.allowedOrigins", []string{})
+ v.SetDefault("server.trustedProxies", []string{})
+ v.SetDefault("server.trustedPlatform", "")
v.SetDefault("db.type", "sqlite")
v.SetDefault("db.dsn", "file:./data/app.db?_busy_timeout=5000")
v.SetDefault("db.maxIdleConns", 5)
@@ -312,6 +391,7 @@ func bindConfigDefaults(v *viper.Viper) {
v.SetDefault("logger.addSource", false)
v.SetDefault("auth.tokenTTLHours", 12)
v.SetDefault("auth.maxFailedAttempts", 5)
+ v.SetDefault("auth.maxFailedAttemptsPerIP", 0)
v.SetDefault("auth.credentialLockMinute", 15)
v.SetDefault("customerSession.ttlMinutes", 120)
v.SetDefault("customerSession.refreshThresholdMinutes", 30)
@@ -336,6 +416,8 @@ func bindEnvironmentAliases(v *viper.Viper) {
_ = v.BindEnv("server.port", "AGENT_DESK_SERVER_PORT", "PORT", "SERVER_PORT")
_ = v.BindEnv("server.companyName", "AGENT_DESK_SERVER_COMPANYNAME", "COMPANY_NAME", "NEXT_PUBLIC_COMPANY_NAME", "BRAND_NAME", "BRAND_COMPANY_NAME")
_ = v.BindEnv("server.companyLogoUrl", "AGENT_DESK_SERVER_COMPANYLOGOURL", "COMPANY_LOGO_URL", "NEXT_PUBLIC_COMPANY_LOGO_URL", "BRAND_LOGO_URL")
+ _ = v.BindEnv("server.trustedProxies", "AGENT_DESK_SERVER_TRUSTEDPROXIES", "TRUSTED_PROXIES")
+ _ = v.BindEnv("server.trustedPlatform", "AGENT_DESK_SERVER_TRUSTEDPLATFORM", "TRUSTED_PLATFORM")
_ = v.BindEnv("db.type", "AGENT_DESK_DB_TYPE", "DATABASE_TYPE", "DB_TYPE")
_ = v.BindEnv("db.dsn", "AGENT_DESK_DB_DSN", "DATABASE_URL", "DB_DSN")
_ = v.BindEnv("auth.passwordLoginEnabled", "AGENT_DESK_AUTH_PASSWORDLOGINENABLED", "PASSWORD_LOGIN_ENABLED")
diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go
index 00a17f95..de1defe5 100644
--- a/internal/pkg/config/config_test.go
+++ b/internal/pkg/config/config_test.go
@@ -156,3 +156,107 @@ ORG_SYNC_SECRET=webhook-secret-789
t.Fatalf("Webhook.OrgSyncSecret=%q", cfg.Webhook.OrgSyncSecret)
}
}
+
+func TestAuthConfigMaxFailedAttemptsPerIPOrDefault(t *testing.T) {
+ cases := []struct {
+ name string
+ cfg AuthConfig
+ want int
+ }{
+ {"explicit value wins", AuthConfig{MaxFailedAttempts: 5, MaxFailedAttemptsPerIP: 30}, 30},
+ {"unset derives four times the per-account limit", AuthConfig{MaxFailedAttempts: 5}, 20},
+ {"disabled per-account limit disables both", AuthConfig{MaxFailedAttempts: 0}, 0},
+ {"negative per-account limit disables both", AuthConfig{MaxFailedAttempts: -1}, 0},
+ }
+ for _, tc := range cases {
+ if got := tc.cfg.MaxFailedAttemptsPerIPOrDefault(); got != tc.want {
+ t.Errorf("%s: MaxFailedAttemptsPerIPOrDefault() = %d want %d", tc.name, got, tc.want)
+ }
+ }
+}
+
+func TestServerConfigTrustedProxiesOrDefault(t *testing.T) {
+ // An unset list must not fall through to Gin's own default of 0.0.0.0/0 and
+ // ::/0, which trusts every peer and makes X-Forwarded-For authoritative.
+ got := ServerConfig{}.TrustedProxiesOrDefault()
+ for _, want := range []string{"127.0.0.0/8", "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "::1/128", "fc00::/7", "fe80::/10"} {
+ found := false
+ for _, entry := range got {
+ if entry == want {
+ found = true
+ }
+ }
+ if !found {
+ t.Errorf("TrustedProxiesOrDefault() = %v, missing %s", got, want)
+ }
+ }
+ for _, entry := range got {
+ if entry == "0.0.0.0/0" || entry == "::/0" {
+ t.Errorf("TrustedProxiesOrDefault() includes %s, which trusts every peer", entry)
+ }
+ }
+
+ // Blank entries from a comma-separated environment variable must not become
+ // empty CIDRs, which Gin would reject at startup.
+ got = ServerConfig{TrustedProxies: []string{" 10.1.0.0/16 ", "", " "}}.TrustedProxiesOrDefault()
+ if len(got) != 1 || got[0] != "10.1.0.0/16" {
+ t.Errorf("TrustedProxiesOrDefault() = %v want [10.1.0.0/16]", got)
+ }
+}
+
+func TestServerConfigTrustedPlatformHeader(t *testing.T) {
+ cases := []struct {
+ platform string
+ want string
+ }{
+ {"", ""},
+ {"cloudflare", "CF-Connecting-IP"},
+ {"Cloudflare", "CF-Connecting-IP"},
+ {" CF ", "CF-Connecting-IP"},
+ {"fly.io", "Fly-Client-IP"},
+ {"google-app-engine", "X-Appengine-Remote-Addr"},
+ // Anything unrecognised is a literal header name, which is what Gin's
+ // TrustedPlatform field expects.
+ {"X-CDN-IP", "X-CDN-IP"},
+ }
+ for _, tc := range cases {
+ if got := (ServerConfig{TrustedPlatform: tc.platform}).TrustedPlatformHeader(); got != tc.want {
+ t.Errorf("TrustedPlatformHeader(%q) = %q want %q", tc.platform, got, tc.want)
+ }
+ }
+}
+
+// TestLoadReadsTrustedProxySettings covers the environment spelling, including
+// the comma-separated list. Gin rejects an unparseable CIDR at startup, so a
+// value that arrives as one string instead of a slice would take the process
+// down rather than degrade quietly.
+func TestLoadReadsTrustedProxySettings(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "config.yaml")
+ if err := os.WriteFile(path, []byte("server:\n port: 8083\n"), 0600); err != nil {
+ t.Fatalf("WriteFile() error = %v", err)
+ }
+
+ t.Setenv("ENV_FILE", os.DevNull)
+ t.Setenv("AGENT_DESK_ENV_FILE", os.DevNull)
+ t.Setenv("TRUSTED_PROXIES", "10.0.0.0/8,172.16.0.0/12")
+ t.Setenv("TRUSTED_PLATFORM", "cloudflare")
+
+ cfg, err := Load(path)
+ if err != nil {
+ t.Fatalf("Load() error = %v", err)
+ }
+
+ want := []string{"10.0.0.0/8", "172.16.0.0/12"}
+ got := cfg.Server.TrustedProxies
+ if len(got) != len(want) {
+ t.Fatalf("TrustedProxies = %v want %v", got, want)
+ }
+ for i := range want {
+ if got[i] != want[i] {
+ t.Fatalf("TrustedProxies[%d] = %q want %q", i, got[i], want[i])
+ }
+ }
+ if cfg.Server.TrustedPlatformHeader() != "CF-Connecting-IP" {
+ t.Errorf("TrustedPlatformHeader() = %q want CF-Connecting-IP", cfg.Server.TrustedPlatformHeader())
+ }
+}
diff --git a/internal/services/auth_service.go b/internal/services/auth_service.go
index 5765768c..4b259459 100644
--- a/internal/services/auth_service.go
+++ b/internal/services/auth_service.go
@@ -90,7 +90,7 @@ func (s *authService) Login(req request.LoginRequest, authCfg config.AuthConfig,
return nil, errorsx.InvalidParamI18n("error.e0258")
}
- if s.isCredentialLocked(principal, authCfg) {
+ if s.isCredentialLocked(principal, clientIP, authCfg) {
_ = s.createLoginCredentialLog(principal, 0, false, clientIP, userAgent, "credential locked")
return nil, errorsx.CredentialLockedI18n("error.e0270")
}
@@ -455,28 +455,71 @@ func (s *authService) createLoginCredentialLog(principal string, userID int64, s
Principal: principal,
UserID: userID,
Success: success,
- ClientIP: clientIP,
+ ClientIP: normalizeClientIP(clientIP),
UserAgent: userAgent,
Reason: reason,
CreatedAt: time.Now(),
})
}
-func (s *authService) isCredentialLocked(principal string, authCfg config.AuthConfig) bool {
- maxFailedAttempts := authCfg.MaxFailedAttempts
- if maxFailedAttempts <= 0 {
- return false
- }
+// isCredentialLocked reports whether this attempt should be refused before the
+// password is even checked.
+//
+// Both windows are keyed on the client address as well as the username. Keying on
+// the username alone - which is what this used to do - means anyone who knows a
+// username can lock the real account out for the whole window, from anywhere, as
+// often as they like: that is a denial of service dressed up as a protection. The
+// principal-and-address window still stops one source grinding on one account, and
+// the address-only window across all principals still stops credential stuffing,
+// but a legitimate user arriving from a different address is no longer locked out
+// by somebody else's failures.
+//
+// This is only sound because the address itself is trustworthy; see the trusted
+// proxy configuration in bootstrap.NewServer.
+func (s *authService) isCredentialLocked(principal, clientIP string, authCfg config.AuthConfig) bool {
lockMinute := authCfg.CredentialLockMinute
if lockMinute <= 0 {
lockMinute = 15
}
since := time.Now().Add(-time.Duration(lockMinute) * time.Minute)
- return LoginCredentialLogService.Count(sqls.NewCnd().
- Eq("principal", normalizeLoginPrincipal(principal)).
- Eq("success", false).
- NotEq("reason", "credential locked").
- Where("created_at >= ?", since)) >= int64(maxFailedAttempts)
+ ip := normalizeClientIP(clientIP)
+
+ base := func() *sqls.Cnd {
+ return sqls.NewCnd().
+ Eq("client_ip", ip).
+ Eq("success", false).
+ NotEq("reason", "credential locked").
+ Where("created_at >= ?", since)
+ }
+
+ if maxFailedAttempts := authCfg.MaxFailedAttempts; maxFailedAttempts > 0 {
+ count := LoginCredentialLogService.Count(base().Eq("principal", normalizeLoginPrincipal(principal)))
+ if count >= int64(maxFailedAttempts) {
+ return true
+ }
+ }
+
+ // An undetermined address would otherwise pool every such caller into one
+ // bucket and lock them all out together.
+ if maxPerIP := authCfg.MaxFailedAttemptsPerIPOrDefault(); maxPerIP > 0 && ip != unknownClientIP {
+ if count := LoginCredentialLogService.Count(base()); count >= int64(maxPerIP) {
+ return true
+ }
+ }
+
+ return false
+}
+
+// unknownClientIP stands in for an address the server could not determine, so
+// those attempts share one bucket on the principal window instead of matching
+// nothing at all.
+const unknownClientIP = "unknown"
+
+func normalizeClientIP(clientIP string) string {
+ if clientIP = strings.TrimSpace(clientIP); clientIP == "" {
+ return unknownClientIP
+ }
+ return clientIP
}
func normalizeLoginPrincipal(principal string) string {
diff --git a/internal/services/auth_service_test.go b/internal/services/auth_service_test.go
index 156db6d5..d137323d 100644
--- a/internal/services/auth_service_test.go
+++ b/internal/services/auth_service_test.go
@@ -112,6 +112,7 @@ func TestAuthServiceLoginCredentialLockout(t *testing.T) {
Principal: "admin",
UserID: user.ID,
Success: false,
+ ClientIP: "127.0.0.1",
Reason: "password mismatch",
CreatedAt: now.Add(-time.Duration(i+1) * time.Minute),
}).Error; err != nil {
@@ -122,6 +123,7 @@ func TestAuthServiceLoginCredentialLockout(t *testing.T) {
Principal: "admin",
UserID: user.ID,
Success: false,
+ ClientIP: "127.0.0.1",
Reason: "password mismatch",
CreatedAt: now.Add(-30 * time.Minute),
}).Error; err != nil {
@@ -164,6 +166,7 @@ func TestAuthServiceCredentialLockoutDoesNotExtendWhileLocked(t *testing.T) {
Principal: "admin",
UserID: 1,
Success: false,
+ ClientIP: "127.0.0.1",
Reason: "password mismatch",
CreatedAt: now.Add(-2 * time.Minute),
},
@@ -171,6 +174,7 @@ func TestAuthServiceCredentialLockoutDoesNotExtendWhileLocked(t *testing.T) {
Principal: "admin",
UserID: 0,
Success: false,
+ ClientIP: "127.0.0.1",
Reason: "credential locked",
CreatedAt: now.Add(-1 * time.Minute),
},
@@ -199,6 +203,7 @@ func TestAuthServiceCredentialLockoutNormalizesPrincipalCase(t *testing.T) {
Principal: "admin",
UserID: 1,
Success: false,
+ ClientIP: "127.0.0.1",
Reason: "password mismatch",
CreatedAt: time.Now().Add(-time.Minute),
}).Error; err != nil {
@@ -223,6 +228,79 @@ func TestAuthServiceCredentialLockoutNormalizesPrincipalCase(t *testing.T) {
}
}
+// TestAuthServiceCredentialLockoutIsScopedToTheClientAddress is the regression
+// test for the denial of service. Keyed on the username alone, anybody who knew a
+// username could lock the real account out for the whole window, from anywhere,
+// as often as they liked.
+func TestAuthServiceCredentialLockoutIsScopedToTheClientAddress(t *testing.T) {
+ db := setupAuthServiceTestDB(t)
+ user := createAuthTestUser(t, db, "admin", "secret")
+ now := time.Now()
+ for i := 0; i < 5; i++ {
+ if err := db.Create(&models.LoginCredentialLog{
+ Principal: "admin",
+ UserID: user.ID,
+ Success: false,
+ ClientIP: "203.0.113.66",
+ Reason: "password mismatch",
+ CreatedAt: now.Add(-time.Duration(i+1) * time.Minute),
+ }).Error; err != nil {
+ t.Fatalf("seed credential log: %v", err)
+ }
+ }
+
+ authCfg := config.AuthConfig{TokenTTLHours: 2, MaxFailedAttempts: 3, CredentialLockMinute: 15}
+
+ if _, err := newAuthService().Login(request.LoginRequest{Username: "admin", Password: "secret"}, authCfg, "203.0.113.66", "go-test"); !hasCode(err, errorsx.CodeAuthCredentialLocked) {
+ t.Fatalf("expected the attacking address to be locked, got %v", err)
+ }
+
+ ret, err := newAuthService().Login(request.LoginRequest{Username: "admin", Password: "secret"}, authCfg, "198.51.100.7", "go-test")
+ if err != nil {
+ t.Fatalf("the legitimate owner was locked out by somebody else's failures: %v", err)
+ }
+ if ret == nil || ret.AccessToken == "" {
+ t.Fatalf("expected a session for the legitimate owner, got %+v", ret)
+ }
+}
+
+// TestAuthServiceCredentialLockoutByAddressAcrossPrincipals covers the window that
+// replaces the removed account-wide lock: one address spraying many usernames.
+func TestAuthServiceCredentialLockoutByAddressAcrossPrincipals(t *testing.T) {
+ db := setupAuthServiceTestDB(t)
+ createAuthTestUser(t, db, "admin", "secret")
+ now := time.Now()
+ for i, principal := range []string{"admin", "root", "operator", "support"} {
+ if err := db.Create(&models.LoginCredentialLog{
+ Principal: principal,
+ UserID: 0,
+ Success: false,
+ ClientIP: "203.0.113.66",
+ Reason: "user not found",
+ CreatedAt: now.Add(-time.Duration(i+1) * time.Minute),
+ }).Error; err != nil {
+ t.Fatalf("seed credential log: %v", err)
+ }
+ }
+
+ authCfg := config.AuthConfig{
+ TokenTTLHours: 2,
+ MaxFailedAttempts: 10,
+ MaxFailedAttemptsPerIP: 4,
+ CredentialLockMinute: 15,
+ }
+
+ // No single username has reached MaxFailedAttempts, so only the per-address
+ // window can catch this.
+ if _, err := newAuthService().Login(request.LoginRequest{Username: "admin", Password: "secret"}, authCfg, "203.0.113.66", "go-test"); !hasCode(err, errorsx.CodeAuthCredentialLocked) {
+ t.Fatalf("expected credential stuffing from one address to be locked, got %v", err)
+ }
+
+ if _, err := newAuthService().Login(request.LoginRequest{Username: "admin", Password: "secret"}, authCfg, "198.51.100.7", "go-test"); err != nil {
+ t.Fatalf("an unrelated address was locked by another address's failures: %v", err)
+ }
+}
+
func TestAuthServiceCredentialLockoutDisabledWhenMaxAttemptsNonPositive(t *testing.T) {
db := setupAuthServiceTestDB(t)
user := createAuthTestUser(t, db, "admin", "secret")
@@ -232,6 +310,7 @@ func TestAuthServiceCredentialLockoutDisabledWhenMaxAttemptsNonPositive(t *testi
Principal: "admin",
UserID: user.ID,
Success: false,
+ ClientIP: "127.0.0.1",
Reason: "credential locked",
CreatedAt: now.Add(-time.Duration(i+1) * time.Minute),
}).Error; err != nil {
From c42d2de28c1848051395d4caf4553216a5997d74 Mon Sep 17 00:00:00 2001
From: JOY <5027251+JOY@users.noreply.github.com>
Date: Sun, 13 Sep 2026 17:15:37 +0700
Subject: [PATCH 6/8] feat(security): rate limit the public endpoints an
anonymous caller can drive
There was no rate limiting anywhere in the application. Every unauthenticated
endpoint could be called in a tight loop, which made three things cheap: flooding
the message upload endpoints with 20 MB bodies, spraying login and support
registration, and burning LLM spend through the conversation endpoints.
Add a small in-process fixed-window limiter and attach it to six public routes:
login (20 per window), support registration (10), the widget session exchange
(120), message attachment and image upload (30, shared between the two), and
documentation feedback (20). The window is 60 seconds by default, and both it and
the on/off switch are configurable.
The limits are sized for a human driving a browser, not for a machine. The session
exchange budget in particular has to absorb a whole support office loading the
widget from behind one NAT address, which is why it sits an order of magnitude
above the others.
Deliberately not limited:
- /api/third/* channel webhooks. A platform that receives a 429 from its webhook
stops retrying and eventually disables the delivery, which takes an entire
channel offline - a far worse outcome than the flood the limit was meant to
stop. Those endpoints authenticate by signature instead.
- /api/ws/* websockets, and /api/webhooks/* which are HMAC verified.
- /api/dashboard/* - already behind AuthMiddleware, and staff sharing one office
address would throttle each other.
Rejections return 429 with a Retry-After header and an ordinary JsonResult body.
The status is a real 429 rather than the 200-with-error-code the auth middleware
uses, because web/lib/api/client.ts parses the payload before it inspects
response.ok and surfaces payload.message, so the localized text still reaches the
user - and Retry-After is only meaningful on a 429 or 503. The message is
error.e0354 in both backend locales and does not disclose the limit or the
remaining budget, which would only tell a caller how much room they have left.
Retry-After rounds the remaining window up rather than down. Telling a caller to
come back sooner than the window actually resets just earns another 429.
Rejections are not logged separately. requestLogMiddleware already records path,
status and client address for every request, so a 429 is visible there without
giving a flood a second way to fill the log.
The limiter keys on ctx.ClientIP(), which is only trustworthy because of the
trusted-proxy configuration that landed immediately before this. Without it a
caller would pick their own bucket with an X-Forwarded-For header.
Counters are per process, and expired buckets are swept lazily from inside Allow
rather than by a goroutine, so there is no background lifetime to own and no
unbounded growth. Running several replicas gives each its own budget, weakening
the bound by the replica count; config.example.yaml says so explicitly. These
limits exist to make flooding expensive, not to meter a quota.
Tests: seven for the limiter, including an exact-count check across 16 goroutines
making 3200 calls against one key; three for the middleware, covering the 429
shape, per-address isolation and a nil limiter allowing everything; and five end
to end against a server built by NewServer, including one that fires 1000 requests
at the health, config, org-sync and two channel webhook routes and fails if any of
them returns 429.
The concurrency test could not be run under -race: this repository builds with CGO
disabled and go test -race requires cgo. It still has teeth, because an unguarded
concurrent map write panics rather than merely miscounting.
---
.env.example | 7 +
config/config.example.yaml | 15 ++
internal/bootstrap/routes.go | 75 ++++++--
internal/bootstrap/server.go | 10 +-
internal/bootstrap/server_ratelimit_test.go | 162 ++++++++++++++++++
internal/middleware/ratelimit_middleware.go | 56 ++++++
.../middleware/ratelimit_middleware_test.go | 103 +++++++++++
internal/pkg/config/config.go | 33 +++-
internal/pkg/config/config_test.go | 51 ++++++
internal/pkg/i18nx/locales/en-US.yml | 1 +
internal/pkg/i18nx/locales/zh-CN.yml | 1 +
internal/pkg/ratelimit/ratelimit.go | 101 +++++++++++
internal/pkg/ratelimit/ratelimit_test.go | 132 ++++++++++++++
13 files changed, 732 insertions(+), 15 deletions(-)
create mode 100644 internal/bootstrap/server_ratelimit_test.go
create mode 100644 internal/middleware/ratelimit_middleware.go
create mode 100644 internal/middleware/ratelimit_middleware_test.go
create mode 100644 internal/pkg/ratelimit/ratelimit.go
create mode 100644 internal/pkg/ratelimit/ratelimit_test.go
diff --git a/.env.example b/.env.example
index edbe2b24..0e8968c1 100644
--- a/.env.example
+++ b/.env.example
@@ -20,6 +20,13 @@ COMPANY_LOGO_URL=
# TRUSTED_PROXIES="10.0.0.0/8,172.16.0.0/12"
TRUSTED_PLATFORM=cloudflare
+# Rate limiting for the public, unauthenticated endpoints (login, support
+# registration, widget session exchange, message uploads, doc feedback). Enabled
+# by default; the counters are per process. Channel webhooks, websockets and the
+# authenticated dashboard are deliberately exempt.
+# RATE_LIMIT_ENABLED=true
+# RATE_LIMIT_WINDOW_SECONDS=60
+
# Database Configuration
# Driver options: sqlite, mysql, postgres
DB_TYPE=sqlite
diff --git a/config/config.example.yaml b/config/config.example.yaml
index d719f25d..91eb2943 100644
--- a/config/config.example.yaml
+++ b/config/config.example.yaml
@@ -17,6 +17,21 @@ server:
# header name) when an edge overwrites rather than appends the real client
# address. When set it takes precedence over X-Forwarded-For entirely.
trustedPlatform: ""
+ rateLimit:
+ # Bounds how often one client address may call the public, unauthenticated
+ # endpoints: login, support registration, the widget session exchange, message
+ # uploads and documentation feedback. Enabled by default.
+ #
+ # Channel webhooks under /api/third, the websocket routes, /api/webhooks and
+ # the whole authenticated dashboard are deliberately NOT covered. A platform
+ # that receives a 429 from its webhook stops retrying and eventually disables
+ # the delivery, which takes an entire channel offline.
+ #
+ # The counters live in this process. Running several replicas gives each one
+ # its own budget, which weakens the bound by the replica count; these limits
+ # exist to make flooding expensive, not to meter a quota.
+ enabled: true
+ windowSeconds: 60
cors:
# Browser CORS allowlist. In production, replace this with the actual frontend or embedded-site domains, such as https://support.example.com.
# Leave it empty to reject cross-origin browser requests. Same-origin and non-browser calls are still supported.
diff --git a/internal/bootstrap/routes.go b/internal/bootstrap/routes.go
index 1482e841..f02307b6 100644
--- a/internal/bootstrap/routes.go
+++ b/internal/bootstrap/routes.go
@@ -1,15 +1,66 @@
package bootstrap
import (
+ "time"
+
"agent-desk/internal/handlers/api"
"agent-desk/internal/handlers/dashboard"
"agent-desk/internal/handlers/third"
+ "agent-desk/internal/middleware"
+ "agent-desk/internal/pkg/config"
+ "agent-desk/internal/pkg/ratelimit"
"github.com/gin-gonic/gin"
)
-func registerApiAuthRoutes(group *gin.RouterGroup) {
- group.POST("/login", api.Login)
+// publicRateLimits holds one limiter per abuse-prone unauthenticated endpoint.
+// They are built once per server rather than per request, and the middleware keys
+// them by client address.
+//
+// Channel webhooks under /api/third, the websocket routes and /api/webhooks are
+// deliberately absent. A platform that receives a 429 from its webhook stops
+// retrying and eventually disables the delivery, which would take a whole channel
+// offline; those endpoints authenticate by signature instead. The dashboard is
+// absent too - it is already behind AuthMiddleware, and staff sharing one office
+// address would throttle each other.
+type publicRateLimits struct {
+ login *ratelimit.Limiter
+ supportRegister *ratelimit.Limiter
+ sessionExchange *ratelimit.Limiter
+ upload *ratelimit.Limiter
+ docFeedback *ratelimit.Limiter
+}
+
+// Limits are sized for a human driving a browser, not for a machine. They are
+// deliberately generous: the point is to make flooding expensive, not to police
+// legitimate use. The session exchange budget in particular has to absorb a whole
+// support office loading the widget from behind one NAT address.
+const (
+ limitLogin = 20
+ limitSupportRegister = 10
+ limitSessionExchange = 120
+ limitUpload = 30
+ limitDocFeedback = 20
+)
+
+func newPublicRateLimits(cfg config.RateLimitConfig) publicRateLimits {
+ if !cfg.IsEnabled() {
+ // Every field stays nil and a nil limiter allows everything, so "disabled"
+ // needs no branch at any call site.
+ return publicRateLimits{}
+ }
+ window := time.Duration(cfg.WindowSecondsOrDefault()) * time.Second
+ return publicRateLimits{
+ login: ratelimit.New(limitLogin, window),
+ supportRegister: ratelimit.New(limitSupportRegister, window),
+ sessionExchange: ratelimit.New(limitSessionExchange, window),
+ upload: ratelimit.New(limitUpload, window),
+ docFeedback: ratelimit.New(limitDocFeedback, window),
+ }
+}
+
+func registerApiAuthRoutes(group *gin.RouterGroup, limits publicRateLimits) {
+ group.POST("/login", middleware.RateLimit(limits.login), api.Login)
group.POST("/logout", api.Logout)
group.GET("/profile", api.Profile)
group.POST("/profile/update", api.UpdateProfile)
@@ -33,8 +84,8 @@ func registerApiWebhookRoutes(group *gin.RouterGroup) {
group.POST("/dos-org-sync", api.DOSOrgSyncWebhook)
}
-func registerApiCustomerRoutes(group *gin.RouterGroup) {
- group.POST("/session_exchange", api.CustomerPostSession_exchange)
+func registerApiCustomerRoutes(group *gin.RouterGroup, limits publicRateLimits) {
+ group.POST("/session_exchange", middleware.RateLimit(limits.sessionExchange), api.CustomerPostSession_exchange)
}
func registerApiConversationRoutes(group *gin.RouterGroup) {
@@ -43,22 +94,26 @@ func registerApiConversationRoutes(group *gin.RouterGroup) {
group.POST("/create_or_match", api.ConversationPostCreate_or_match)
}
-func registerApiMessageRoutes(group *gin.RouterGroup) {
+func registerApiMessageRoutes(group *gin.RouterGroup, limits publicRateLimits) {
group.Any("/list", api.MessageAnyList)
group.POST("/read", api.MessagePostRead)
group.POST("/send", api.MessagePostSend)
- group.POST("/upload_attachment", api.MessagePostUpload_attachment)
- group.POST("/upload_image", api.MessagePostUpload_image)
+ // One shared budget for both upload routes: what matters is how many bytes an
+ // unauthenticated caller can push at the storage layer, not which of the two
+ // endpoints they used.
+ uploadLimit := middleware.RateLimit(limits.upload)
+ group.POST("/upload_attachment", uploadLimit, api.MessagePostUpload_attachment)
+ group.POST("/upload_image", uploadLimit, api.MessagePostUpload_image)
}
-func registerApiSupportRoutes(group *gin.RouterGroup) {
+func registerApiSupportRoutes(group *gin.RouterGroup, limits publicRateLimits) {
group.GET("/config", api.SupportConfigGetConfig)
- group.POST("/auth/register", api.SupportAuthPostRegister)
+ group.POST("/auth/register", middleware.RateLimit(limits.supportRegister), api.SupportAuthPostRegister)
group.GET("/me", api.SupportGetMe)
group.Any("/doc-page/list", api.DocPageAnyList)
group.GET("/doc-page/navigation", api.DocPageGetNavigation)
group.GET("/doc-page/:id", api.DocPageGetBy)
- group.POST("/doc-page/feedback", api.DocPagePostFeedback)
+ group.POST("/doc-page/feedback", middleware.RateLimit(limits.docFeedback), api.DocPagePostFeedback)
group.Any("/community/categories/list", api.CategoryAnyList)
group.Any("/community/posts/list", api.PostAnyList)
group.GET("/community/posts/:id", api.PostGetBy)
diff --git a/internal/bootstrap/server.go b/internal/bootstrap/server.go
index 16416620..49d79580 100644
--- a/internal/bootstrap/server.go
+++ b/internal/bootstrap/server.go
@@ -171,17 +171,19 @@ func isWebsocketUpgrade(ctx *gin.Context) bool {
}
func addRouter(app *gin.Engine) {
+ limits := newPublicRateLimits(config.Current().Server.RateLimit)
+
app.Any("/api/mcp", gin.WrapH(mcps.NewHTTPHandler()))
apiGroup := app.Group("/api")
apiGroup.GET("/health", api.Health)
apiGroup.GET("/config", api.PublicConfig)
- registerApiAuthRoutes(apiGroup.Group("/auth"))
+ registerApiAuthRoutes(apiGroup.Group("/auth"), limits)
registerApiChannelRoutes(apiGroup.Group("/channel"))
- registerApiCustomerRoutes(apiGroup.Group("/customer"))
+ registerApiCustomerRoutes(apiGroup.Group("/customer"), limits)
registerApiConversationRoutes(apiGroup.Group("/conversation", middleware.ExternalUserMiddleware))
- registerApiMessageRoutes(apiGroup.Group("/message", middleware.ExternalUserMiddleware))
- registerApiSupportRoutes(apiGroup.Group("/support"))
+ registerApiMessageRoutes(apiGroup.Group("/message", middleware.ExternalUserMiddleware), limits)
+ registerApiSupportRoutes(apiGroup.Group("/support"), limits)
registerApiWebhookRoutes(apiGroup.Group("/webhooks"))
wsGroup := app.Group("/api/ws")
diff --git a/internal/bootstrap/server_ratelimit_test.go b/internal/bootstrap/server_ratelimit_test.go
new file mode 100644
index 00000000..f1ae3f14
--- /dev/null
+++ b/internal/bootstrap/server_ratelimit_test.go
@@ -0,0 +1,162 @@
+package bootstrap
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "strconv"
+ "strings"
+ "testing"
+
+ "agent-desk/internal/pkg/config"
+
+ "github.com/gin-gonic/gin"
+)
+
+func parseInt(t *testing.T, value string) int {
+ t.Helper()
+ parsed, err := strconv.Atoi(value)
+ if err != nil {
+ t.Fatalf("%q is not an integer", value)
+ }
+ return parsed
+}
+
+// setRateLimitTestConfig disables password login so /api/auth/login returns
+// before it reaches the database. The limiter runs ahead of the handler either
+// way, which is what these tests are about.
+func setRateLimitTestConfig(rateLimit config.RateLimitConfig) {
+ disabled := false
+ config.SetCurrent(&config.Config{
+ Server: config.ServerConfig{
+ RateLimit: rateLimit,
+ CORS: config.CORSConfig{AllowedOrigins: []string{}},
+ },
+ Auth: config.AuthConfig{PasswordLoginEnabled: &disabled},
+ Storage: config.StorageConfig{Local: config.LocalStorageConfig{Root: "storage", BaseURL: "/storage"}},
+ })
+}
+
+func postJSON(app *gin.Engine, path, clientIP string) *httptest.ResponseRecorder {
+ rec := httptest.NewRecorder()
+ req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(`{"username":"admin","password":"secret"}`))
+ req.Header.Set("Content-Type", "application/json")
+ req.RemoteAddr = clientIP + ":52000"
+ app.ServeHTTP(rec, req)
+ return rec
+}
+
+func TestNewServerRateLimitsThePublicLoginEndpoint(t *testing.T) {
+ setRateLimitTestConfig(config.RateLimitConfig{})
+ app, err := NewServer()
+ if err != nil {
+ t.Fatalf("NewServer() error = %v", err)
+ }
+
+ for i := 1; i <= limitLogin; i++ {
+ if rec := postJSON(app, "/api/auth/login", "203.0.113.7"); rec.Code == http.StatusTooManyRequests {
+ t.Fatalf("request %d was throttled although the login limit is %d per window", i, limitLogin)
+ }
+ }
+ rec := postJSON(app, "/api/auth/login", "203.0.113.7")
+ if rec.Code != http.StatusTooManyRequests {
+ t.Fatalf("request %d got status %d, want 429", limitLogin+1, rec.Code)
+ }
+ if rec.Header().Get("Retry-After") == "" {
+ t.Error("the 429 carried no Retry-After header")
+ }
+
+ // A second address must keep working, otherwise one attacker could lock the
+ // login page for every user behind their own NAT.
+ if rec := postJSON(app, "/api/auth/login", "198.51.100.9"); rec.Code == http.StatusTooManyRequests {
+ t.Fatal("an unrelated address was throttled by another address's requests")
+ }
+}
+
+func TestNewServerRateLimitsTheSupportRegistrationEndpoint(t *testing.T) {
+ setRateLimitTestConfig(config.RateLimitConfig{})
+ app, err := NewServer()
+ if err != nil {
+ t.Fatalf("NewServer() error = %v", err)
+ }
+
+ for i := 1; i <= limitSupportRegister; i++ {
+ postJSON(app, "/api/support/auth/register", "203.0.113.7")
+ }
+ if rec := postJSON(app, "/api/support/auth/register", "203.0.113.7"); rec.Code != http.StatusTooManyRequests {
+ t.Fatalf("registration attempt %d got status %d, want 429", limitSupportRegister+1, rec.Code)
+ }
+}
+
+// TestNewServerLeavesWebhooksAndPublicReadsUnthrottled guards the exemption that
+// matters most. A channel platform that receives a 429 from its webhook stops
+// retrying and eventually disables the delivery, which takes a whole channel
+// offline - a far worse outcome than the flood the limit was meant to stop.
+func TestNewServerLeavesWebhooksAndPublicReadsUnthrottled(t *testing.T) {
+ setRateLimitTestConfig(config.RateLimitConfig{})
+ app, err := NewServer()
+ if err != nil {
+ t.Fatalf("NewServer() error = %v", err)
+ }
+
+ requests := []struct {
+ method string
+ path string
+ }{
+ {http.MethodGet, "/api/health"},
+ {http.MethodGet, "/api/config"},
+ {http.MethodPost, "/api/webhooks/org-sync"},
+ {http.MethodGet, "/api/third/telegram/webhook"},
+ {http.MethodPost, "/api/third/whatsapp/webhook"},
+ }
+ for _, r := range requests {
+ for i := 0; i < 200; i++ {
+ rec := httptest.NewRecorder()
+ req := httptest.NewRequest(r.method, r.path, strings.NewReader(`{}`))
+ req.Header.Set("Content-Type", "application/json")
+ req.RemoteAddr = "203.0.113.7:52000"
+ app.ServeHTTP(rec, req)
+ // Any status is acceptable here except 429: these routes either
+ // succeed, fail validation, or error out, but they must never be
+ // throttled.
+ if rec.Code == http.StatusTooManyRequests {
+ t.Fatalf("%s %s returned 429 on request %d; this route must stay unthrottled", r.method, r.path, i+1)
+ }
+ }
+ }
+}
+
+func TestNewServerRateLimitCanBeDisabled(t *testing.T) {
+ disabled := false
+ setRateLimitTestConfig(config.RateLimitConfig{Enabled: &disabled})
+ app, err := NewServer()
+ if err != nil {
+ t.Fatalf("NewServer() error = %v", err)
+ }
+
+ for i := 0; i < limitLogin*4; i++ {
+ if rec := postJSON(app, "/api/auth/login", "203.0.113.7"); rec.Code == http.StatusTooManyRequests {
+ t.Fatalf("request %d was throttled although rate limiting is disabled", i+1)
+ }
+ }
+}
+
+func TestNewServerRateLimitWindowIsConfigurable(t *testing.T) {
+ setRateLimitTestConfig(config.RateLimitConfig{WindowSeconds: 3600})
+ app, err := NewServer()
+ if err != nil {
+ t.Fatalf("NewServer() error = %v", err)
+ }
+
+ for i := 1; i <= limitLogin; i++ {
+ postJSON(app, "/api/auth/login", "203.0.113.7")
+ }
+ rec := postJSON(app, "/api/auth/login", "203.0.113.7")
+ if rec.Code != http.StatusTooManyRequests {
+ t.Fatalf("got status %d, want 429", rec.Code)
+ }
+ if retryAfter := rec.Header().Get("Retry-After"); retryAfter == "" {
+ t.Fatal("no Retry-After header")
+ } else if seconds := parseInt(t, retryAfter); seconds < 3500 || seconds > 3600 {
+ t.Errorf("Retry-After = %s, want roughly the configured 3600 second window", retryAfter)
+ }
+}
diff --git a/internal/middleware/ratelimit_middleware.go b/internal/middleware/ratelimit_middleware.go
new file mode 100644
index 00000000..506b31b7
--- /dev/null
+++ b/internal/middleware/ratelimit_middleware.go
@@ -0,0 +1,56 @@
+package middleware
+
+import (
+ "net/http"
+ "strconv"
+ "time"
+
+ "agent-desk/internal/pkg/errorsx"
+ "agent-desk/internal/pkg/httpx"
+ "agent-desk/internal/pkg/ratelimit"
+
+ "github.com/gin-gonic/gin"
+)
+
+// rateLimitMessageKey is the localized explanation returned with a 429. It
+// deliberately does not disclose the limit or the remaining budget, which would
+// only tell a caller how much room they have left.
+const rateLimitMessageKey = "error.e0354"
+
+// RateLimit bounds how often one client address may call a single public
+// endpoint.
+//
+// The address comes from ctx.ClientIP(), which is only meaningful because
+// NewServer configures Gin's trusted proxies and platform before any middleware
+// is registered. Without that a caller picks their own bucket by setting
+// X-Forwarded-For, and the limit is decoration.
+//
+// A nil limiter allows everything, so "disabled" is expressed by handing out nil
+// limiters rather than by branching at every call site.
+//
+// Rejections are not logged here. requestLogMiddleware already records the path,
+// status and client address of every request, so a 429 is visible there without
+// giving a flood a second way to fill the log.
+func RateLimit(limiter *ratelimit.Limiter) gin.HandlerFunc {
+ return func(ctx *gin.Context) {
+ allowed, retryAfter := limiter.Allow(ctx.ClientIP())
+ if allowed {
+ ctx.Next()
+ return
+ }
+ // Retry-After is a whole number of seconds, so round up: telling a caller
+ // to come back sooner than the window actually resets just earns another
+ // 429. Rounding down and adding one overshoots when the remaining time is
+ // already an exact number of seconds.
+ seconds := int64(retryAfter / time.Second)
+ if time.Duration(seconds)*time.Second < retryAfter {
+ seconds++
+ }
+ if seconds < 1 {
+ seconds = 1
+ }
+ ctx.Header("Retry-After", strconv.FormatInt(seconds, 10))
+ httpx.WriteHttpStatusJSON(ctx, http.StatusTooManyRequests, errorsx.InvalidParamI18n(rateLimitMessageKey))
+ ctx.Abort()
+ }
+}
diff --git a/internal/middleware/ratelimit_middleware_test.go b/internal/middleware/ratelimit_middleware_test.go
new file mode 100644
index 00000000..e2aa1cbf
--- /dev/null
+++ b/internal/middleware/ratelimit_middleware_test.go
@@ -0,0 +1,103 @@
+package middleware
+
+import (
+ "encoding/json"
+ "net/http"
+ "net/http/httptest"
+ "strconv"
+ "testing"
+ "time"
+
+ "agent-desk/internal/pkg/ratelimit"
+
+ "github.com/gin-gonic/gin"
+)
+
+func newRateLimitTestEngine(limiter *ratelimit.Limiter) *gin.Engine {
+ gin.SetMode(gin.TestMode)
+ app := gin.New()
+ app.POST("/probe", RateLimit(limiter), func(ctx *gin.Context) {
+ ctx.JSON(http.StatusOK, gin.H{"success": true})
+ })
+ return app
+}
+
+func postProbe(app *gin.Engine, clientIP string) *httptest.ResponseRecorder {
+ rec := httptest.NewRecorder()
+ req := httptest.NewRequest(http.MethodPost, "/probe", nil)
+ req.RemoteAddr = clientIP + ":52000"
+ app.ServeHTTP(rec, req)
+ return rec
+}
+
+func TestRateLimitRefusesAfterTheLimitAndSetsRetryAfter(t *testing.T) {
+ app := newRateLimitTestEngine(ratelimit.New(2, time.Minute))
+
+ for i := 1; i <= 2; i++ {
+ if rec := postProbe(app, "203.0.113.7"); rec.Code != http.StatusOK {
+ t.Fatalf("request %d got status %d, want 200", i, rec.Code)
+ }
+ }
+
+ rec := postProbe(app, "203.0.113.7")
+ if rec.Code != http.StatusTooManyRequests {
+ t.Fatalf("the third request got status %d, want 429", rec.Code)
+ }
+
+ retryAfter := rec.Header().Get("Retry-After")
+ if retryAfter == "" {
+ t.Fatal("a 429 without Retry-After gives the caller nothing to act on")
+ }
+ seconds, err := strconv.Atoi(retryAfter)
+ if err != nil {
+ t.Fatalf("Retry-After = %q is not a number of seconds", retryAfter)
+ }
+ // The window is one minute and the requests are microseconds apart, so the
+ // answer is 60 give or take a clock tick. What must not happen is a value
+ // above the window, which would tell the caller to wait longer than needed, or
+ // zero, which would have them retry immediately and collect another 429.
+ if seconds < 1 || seconds > 60 {
+ t.Errorf("Retry-After = %d seconds, want between 1 and the 60 second window", seconds)
+ }
+
+ // The body has to stay a JsonResult, because web/lib/api/client.ts parses the
+ // payload before it looks at response.ok and surfaces payload.message.
+ var body struct {
+ Success bool `json:"success"`
+ Message string `json:"message"`
+ }
+ if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
+ t.Fatalf("429 body is not JSON: %v (%s)", err, rec.Body.String())
+ }
+ if body.Success {
+ t.Error("429 body reported success=true")
+ }
+ if body.Message == "" {
+ t.Error("429 body carried no message, so the caller would see a blank error")
+ }
+}
+
+func TestRateLimitIsKeyedByClientAddress(t *testing.T) {
+ app := newRateLimitTestEngine(ratelimit.New(1, time.Minute))
+
+ if rec := postProbe(app, "203.0.113.7"); rec.Code != http.StatusOK {
+ t.Fatalf("first request got %d, want 200", rec.Code)
+ }
+ if rec := postProbe(app, "203.0.113.7"); rec.Code != http.StatusTooManyRequests {
+ t.Fatalf("second request from the same address got %d, want 429", rec.Code)
+ }
+ // Somebody else must not inherit the first caller's exhaustion. Behind a NAT
+ // this is the difference between a working product and a shared lockout.
+ if rec := postProbe(app, "198.51.100.9"); rec.Code != http.StatusOK {
+ t.Fatalf("a different address got %d, want 200", rec.Code)
+ }
+}
+
+func TestRateLimitWithNilLimiterAllowsEverything(t *testing.T) {
+ app := newRateLimitTestEngine(nil)
+ for i := 0; i < 50; i++ {
+ if rec := postProbe(app, "203.0.113.7"); rec.Code != http.StatusOK {
+ t.Fatalf("request %d got %d with a nil limiter, want 200", i, rec.Code)
+ }
+ }
+}
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index 8d224a64..ef17610e 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -60,7 +60,8 @@ type ServerConfig struct {
// TrustedPlatform names an edge that overwrites rather than appends the real
// client address, for example "cloudflare". When set it takes precedence over
// X-Forwarded-For entirely.
- TrustedPlatform string `yaml:"trustedPlatform"`
+ TrustedPlatform string `yaml:"trustedPlatform"`
+ RateLimit RateLimitConfig `yaml:"rateLimit"`
}
// defaultTrustedProxies covers loopback, RFC1918, IPv6 unique-local and
@@ -126,6 +127,33 @@ type CORSConfig struct {
AllowedOrigins []string `yaml:"allowedOrigins"`
}
+// RateLimitConfig bounds how often one client address may call the public,
+// unauthenticated endpoints. It deliberately does not cover channel webhooks,
+// websockets or authenticated dashboard routes: a platform that receives a 429
+// from a webhook endpoint stops retrying and eventually disables the webhook.
+type RateLimitConfig struct {
+ // Enabled defaults to true. Set it to false to switch the limits off without
+ // taking them out of the route table.
+ Enabled *bool `yaml:"enabled"`
+ // WindowSeconds is the length of the counting window. Zero or negative means
+ // one minute.
+ WindowSeconds int `yaml:"windowSeconds"`
+}
+
+func (r RateLimitConfig) IsEnabled() bool {
+ if r.Enabled == nil {
+ return true
+ }
+ return *r.Enabled
+}
+
+func (r RateLimitConfig) WindowSecondsOrDefault() int {
+ if r.WindowSeconds <= 0 {
+ return 60
+ }
+ return r.WindowSeconds
+}
+
type DBConfig struct {
Type string `yaml:"type"`
DSN string `yaml:"dsn"`
@@ -380,6 +408,7 @@ func bindConfigDefaults(v *viper.Viper) {
v.SetDefault("server.cors.allowedOrigins", []string{})
v.SetDefault("server.trustedProxies", []string{})
v.SetDefault("server.trustedPlatform", "")
+ v.SetDefault("server.rateLimit.windowSeconds", 60)
v.SetDefault("db.type", "sqlite")
v.SetDefault("db.dsn", "file:./data/app.db?_busy_timeout=5000")
v.SetDefault("db.maxIdleConns", 5)
@@ -418,6 +447,8 @@ func bindEnvironmentAliases(v *viper.Viper) {
_ = v.BindEnv("server.companyLogoUrl", "AGENT_DESK_SERVER_COMPANYLOGOURL", "COMPANY_LOGO_URL", "NEXT_PUBLIC_COMPANY_LOGO_URL", "BRAND_LOGO_URL")
_ = v.BindEnv("server.trustedProxies", "AGENT_DESK_SERVER_TRUSTEDPROXIES", "TRUSTED_PROXIES")
_ = v.BindEnv("server.trustedPlatform", "AGENT_DESK_SERVER_TRUSTEDPLATFORM", "TRUSTED_PLATFORM")
+ _ = v.BindEnv("server.rateLimit.enabled", "AGENT_DESK_SERVER_RATELIMIT_ENABLED", "RATE_LIMIT_ENABLED")
+ _ = v.BindEnv("server.rateLimit.windowSeconds", "AGENT_DESK_SERVER_RATELIMIT_WINDOWSECONDS", "RATE_LIMIT_WINDOW_SECONDS")
_ = v.BindEnv("db.type", "AGENT_DESK_DB_TYPE", "DATABASE_TYPE", "DB_TYPE")
_ = v.BindEnv("db.dsn", "AGENT_DESK_DB_DSN", "DATABASE_URL", "DB_DSN")
_ = v.BindEnv("auth.passwordLoginEnabled", "AGENT_DESK_AUTH_PASSWORDLOGINENABLED", "PASSWORD_LOGIN_ENABLED")
diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go
index de1defe5..e6874258 100644
--- a/internal/pkg/config/config_test.go
+++ b/internal/pkg/config/config_test.go
@@ -260,3 +260,54 @@ func TestLoadReadsTrustedProxySettings(t *testing.T) {
t.Errorf("TrustedPlatformHeader() = %q want CF-Connecting-IP", cfg.Server.TrustedPlatformHeader())
}
}
+
+func TestRateLimitConfigDefaultsToEnabledWithAMinuteWindow(t *testing.T) {
+ var cfg RateLimitConfig
+ if !cfg.IsEnabled() {
+ t.Error("rate limiting must default to enabled; a zero-value config should not silently turn protection off")
+ }
+ if got := cfg.WindowSecondsOrDefault(); got != 60 {
+ t.Errorf("WindowSecondsOrDefault() = %d want 60", got)
+ }
+
+ off := false
+ cfg = RateLimitConfig{Enabled: &off, WindowSeconds: -5}
+ if cfg.IsEnabled() {
+ t.Error("Enabled=false must disable the limits")
+ }
+ if got := cfg.WindowSecondsOrDefault(); got != 60 {
+ t.Errorf("a non-positive window must fall back to 60, got %d", got)
+ }
+
+ on := true
+ cfg = RateLimitConfig{Enabled: &on, WindowSeconds: 30}
+ if !cfg.IsEnabled() {
+ t.Error("Enabled=true must enable the limits")
+ }
+ if got := cfg.WindowSecondsOrDefault(); got != 30 {
+ t.Errorf("WindowSecondsOrDefault() = %d want 30", got)
+ }
+}
+
+func TestLoadReadsRateLimitSettings(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "config.yaml")
+ if err := os.WriteFile(path, []byte("server:\n port: 8083\n"), 0600); err != nil {
+ t.Fatalf("WriteFile() error = %v", err)
+ }
+
+ t.Setenv("ENV_FILE", os.DevNull)
+ t.Setenv("AGENT_DESK_ENV_FILE", os.DevNull)
+ t.Setenv("RATE_LIMIT_ENABLED", "false")
+ t.Setenv("RATE_LIMIT_WINDOW_SECONDS", "30")
+
+ cfg, err := Load(path)
+ if err != nil {
+ t.Fatalf("Load() error = %v", err)
+ }
+ if cfg.Server.RateLimit.IsEnabled() {
+ t.Error("RATE_LIMIT_ENABLED=false did not disable the limits")
+ }
+ if got := cfg.Server.RateLimit.WindowSecondsOrDefault(); got != 30 {
+ t.Errorf("WindowSecondsOrDefault() = %d want 30", got)
+ }
+}
diff --git a/internal/pkg/i18nx/locales/en-US.yml b/internal/pkg/i18nx/locales/en-US.yml
index e74b751b..c07c8d97 100644
--- a/internal/pkg/i18nx/locales/en-US.yml
+++ b/internal/pkg/i18nx/locales/en-US.yml
@@ -346,6 +346,7 @@ error.e0345: "Attachment message is missing assetId."
error.e0346: "Attachment message is missing payload."
error.e0347: "Default team queue mode requires at least one agent team."
error.e0348: "This file type cannot be uploaded for security reasons."
+error.e0354: "Too many requests. Please wait a moment and try again."
error.profile.nicknameRequired: "Enter a nickname."
error.profile.nicknameTooLong: "Nickname cannot exceed 100 characters."
error.profile.avatarTooLong: "Avatar link cannot exceed 255 characters."
diff --git a/internal/pkg/i18nx/locales/zh-CN.yml b/internal/pkg/i18nx/locales/zh-CN.yml
index 371264f0..457540e5 100644
--- a/internal/pkg/i18nx/locales/zh-CN.yml
+++ b/internal/pkg/i18nx/locales/zh-CN.yml
@@ -346,6 +346,7 @@ error.e0345: "附件消息缺少 assetId"
error.e0346: "附件消息缺少 payload"
error.e0347: "默认客服组待接入池模式必须至少选择一个客服组"
error.e0348: "出于安全考虑，此文件类型不支持上传"
+error.e0354: "请求过于频繁，请稍后再试。"
error.profile.nicknameRequired: "请输入昵称"
error.profile.nicknameTooLong: "昵称不能超过 100 个字符"
error.profile.avatarTooLong: "头像链接不能超过 255 个字符"
diff --git a/internal/pkg/ratelimit/ratelimit.go b/internal/pkg/ratelimit/ratelimit.go
new file mode 100644
index 00000000..47f4e6ea
--- /dev/null
+++ b/internal/pkg/ratelimit/ratelimit.go
@@ -0,0 +1,101 @@
+// Package ratelimit provides a small in-process fixed-window counter for the
+// public endpoints that an unauthenticated caller can drive.
+//
+// It is deliberately not distributed. A deployment running several replicas gets
+// the configured limit per replica rather than per cluster, which weakens the
+// bound by a factor of the replica count. That is the right trade here because
+// these limits exist to make flooding expensive, not to meter a quota, and
+// because reaching for a shared store would put a network round trip on the
+// hottest unauthenticated paths in the application. If this ever needs to be
+// exact across replicas, the Limiter interface is small enough to swap.
+package ratelimit
+
+import (
+ "sync"
+ "time"
+)
+
+type bucket struct {
+ count int
+ resetAt time.Time
+}
+
+// Limiter counts requests per key inside a fixed window. The zero value is not
+// usable; call New. A nil *Limiter allows everything, so a caller can represent
+// "disabled" by holding nil instead of branching at every use site.
+type Limiter struct {
+ limit int
+ window time.Duration
+
+ mu sync.Mutex
+ buckets map[string]bucket
+ sweptAt time.Time
+}
+
+// New returns a Limiter that allows limit requests per window for each distinct
+// key. A limit of zero or less, or a non-positive window, disables the limiter.
+func New(limit int, window time.Duration) *Limiter {
+ if limit <= 0 || window <= 0 {
+ return &Limiter{limit: limit, window: window, buckets: make(map[string]bucket)}
+ }
+ return &Limiter{limit: limit, window: window, buckets: make(map[string]bucket)}
+}
+
+// Allow records one request for key and reports whether it may proceed. When it
+// returns false, retryAfter is how long the caller should wait before the window
+// resets, and is meant for a Retry-After header.
+func (l *Limiter) Allow(key string) (bool, time.Duration) {
+ if l == nil || l.limit <= 0 || l.window <= 0 {
+ return true, 0
+ }
+
+ now := time.Now()
+
+ l.mu.Lock()
+ defer l.mu.Unlock()
+
+ l.sweep(now)
+
+ current, ok := l.buckets[key]
+ if !ok || !now.Before(current.resetAt) {
+ l.buckets[key] = bucket{count: 1, resetAt: now.Add(l.window)}
+ return true, 0
+ }
+
+ current.count++
+ l.buckets[key] = current
+ if current.count > l.limit {
+ retryAfter := current.resetAt.Sub(now)
+ if retryAfter < 0 {
+ retryAfter = 0
+ }
+ return false, retryAfter
+ }
+ return true, 0
+}
+
+// Len reports how many keys are currently tracked. It exists for tests.
+func (l *Limiter) Len() int {
+ if l == nil {
+ return 0
+ }
+ l.mu.Lock()
+ defer l.mu.Unlock()
+ return len(l.buckets)
+}
+
+// sweep drops buckets whose window has elapsed, so a long-running process does
+// not accumulate one entry per address it has ever seen. It runs at most once per
+// window and under the same lock as Allow, which keeps the cost amortised and
+// avoids a background goroutine whose lifetime somebody would have to own.
+func (l *Limiter) sweep(now time.Time) {
+ if !l.sweptAt.IsZero() && now.Sub(l.sweptAt) < l.window {
+ return
+ }
+ for key, current := range l.buckets {
+ if !now.Before(current.resetAt) {
+ delete(l.buckets, key)
+ }
+ }
+ l.sweptAt = now
+}
diff --git a/internal/pkg/ratelimit/ratelimit_test.go b/internal/pkg/ratelimit/ratelimit_test.go
new file mode 100644
index 00000000..dfa78ea6
--- /dev/null
+++ b/internal/pkg/ratelimit/ratelimit_test.go
@@ -0,0 +1,132 @@
+package ratelimit
+
+import (
+ "fmt"
+ "sync"
+ "sync/atomic"
+ "testing"
+ "time"
+)
+
+func TestLimiterAllowsUpToTheLimitThenRefuses(t *testing.T) {
+ l := New(3, time.Minute)
+ for i := 1; i <= 3; i++ {
+ if ok, retryAfter := l.Allow("203.0.113.7"); !ok {
+ t.Fatalf("request %d was refused although the limit is 3 (retryAfter=%v)", i, retryAfter)
+ }
+ }
+
+ ok, retryAfter := l.Allow("203.0.113.7")
+ if ok {
+ t.Fatal("the fourth request was allowed although the limit is 3")
+ }
+ if retryAfter <= 0 || retryAfter > time.Minute {
+ t.Fatalf("retryAfter = %v, want a positive duration within the window", retryAfter)
+ }
+}
+
+func TestLimiterKeysAreIndependent(t *testing.T) {
+ l := New(1, time.Minute)
+ if ok, _ := l.Allow("203.0.113.7"); !ok {
+ t.Fatal("the first request for a key was refused")
+ }
+ if ok, _ := l.Allow("203.0.113.7"); ok {
+ t.Fatal("the second request for the same key was allowed")
+ }
+ // A different caller must not inherit somebody else's exhaustion. Without key
+ // isolation one blocked address would lock out every user behind it.
+ if ok, _ := l.Allow("198.51.100.9"); !ok {
+ t.Fatal("a different key was refused because another key was exhausted")
+ }
+}
+
+func TestLimiterWindowResets(t *testing.T) {
+ l := New(1, 20*time.Millisecond)
+ if ok, _ := l.Allow("k"); !ok {
+ t.Fatal("the first request was refused")
+ }
+ if ok, _ := l.Allow("k"); ok {
+ t.Fatal("the second request inside the window was allowed")
+ }
+ time.Sleep(40 * time.Millisecond)
+ if ok, _ := l.Allow("k"); !ok {
+ t.Fatal("the window did not reset")
+ }
+}
+
+func TestNilLimiterAllowsEverything(t *testing.T) {
+ var l *Limiter
+ for i := 0; i < 100; i++ {
+ if ok, retryAfter := l.Allow("k"); !ok {
+ t.Fatalf("a nil limiter refused request %d (retryAfter=%v)", i, retryAfter)
+ }
+ }
+ if l.Len() != 0 {
+ t.Fatalf("Len() on a nil limiter = %d want 0", l.Len())
+ }
+}
+
+func TestDisabledLimiterAllowsEverything(t *testing.T) {
+ cases := []struct {
+ name string
+ limiter *Limiter
+ }{
+ {"zero limit", New(0, time.Minute)},
+ {"negative limit", New(-1, time.Minute)},
+ {"zero window", New(5, 0)},
+ {"negative window", New(5, -time.Second)},
+ }
+ for _, tc := range cases {
+ for i := 0; i < 50; i++ {
+ if ok, _ := tc.limiter.Allow("k"); !ok {
+ t.Fatalf("%s: request %d was refused although the limiter is disabled", tc.name, i)
+ }
+ }
+ }
+}
+
+func TestSweepReleasesExpiredKeys(t *testing.T) {
+ l := New(10, 20*time.Millisecond)
+ for i := 0; i < 200; i++ {
+ l.Allow(fmt.Sprintf("key-%d", i))
+ }
+ if got := l.Len(); got != 200 {
+ t.Fatalf("Len() = %d before the sweep, want 200", got)
+ }
+
+ time.Sleep(40 * time.Millisecond)
+ // The sweep is driven by Allow rather than by a goroutine, so one more call
+ // has to reclaim everything that expired.
+ l.Allow("fresh")
+ if got := l.Len(); got != 1 {
+ t.Fatalf("Len() = %d after the sweep, want only the fresh key", got)
+ }
+}
+
+// TestLimiterCountsExactlyUnderConcurrency is the reason Allow holds a single
+// mutex across the read-modify-write rather than checking and then recording.
+func TestLimiterCountsExactlyUnderConcurrency(t *testing.T) {
+ const goroutines = 16
+ const perGoroutine = 200
+ const limit = 1000
+
+ l := New(limit, time.Minute)
+ var allowed atomic.Int64
+ var wg sync.WaitGroup
+ for i := 0; i < goroutines; i++ {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ for j := 0; j < perGoroutine; j++ {
+ if ok, _ := l.Allow("shared"); ok {
+ allowed.Add(1)
+ }
+ }
+ }()
+ }
+ wg.Wait()
+
+ if got := allowed.Load(); got != limit {
+ t.Fatalf("allowed %d of %d requests, want exactly the limit of %d", got, goroutines*perGoroutine, limit)
+ }
+}
From 7ebed455964f78473c668f8f37345e6505bb71d6 Mon Sep 17 00:00:00 2001
From: JOY <5027251+JOY@users.noreply.github.com>
Date: Tue, 22 Sep 2026 10:42:16 +0700
Subject: [PATCH 7/8] feat(auth): auto-redirect to the SSO provider when it is
the only transport
When OIDC is enabled, password login is disabled, and no other provider
is configured, /dashboard/login now sends the visitor straight to the
provider instead of rendering a page whose only working control is the
"sign in with SSO" button. The redirect waits for the session probe, so
an already-signed-in visitor still lands on their destination, and it is
suppressed when the provider bounced back with ?oidcError= so a failing
IdP cannot create a redirect loop (the page then renders with its error
toast and the manual button), and in WxWork-only environments.
---
web/components/login-form.tsx | 37 ++++++++++++++++++++++++++++++++++-
web/messages/en-US.json | 1 +
web/messages/zh-CN.json | 1 +
3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/web/components/login-form.tsx b/web/components/login-form.tsx
index 34bb9ad4..499bb365 100644
--- a/web/components/login-form.tsx
+++ b/web/components/login-form.tsx
@@ -38,7 +38,7 @@ export function LoginForm({
const t = useI18n()
const router = useRouter()
const searchParams = useSearchParams()
- const { session } = useAuth()
+ const { session, ready } = useAuth()
const [isPending, setIsPending] = useState(false)
const [isWxWorkEnv, setIsWxWorkEnv] = useState(false)
const [publicConfig, setPublicConfig] = useState(null)
@@ -52,6 +52,28 @@ export function LoginForm({
Number(publicConfig?.wxworkEnabled) + Number(publicConfig?.oidcEnabled)
const isPasswordLoginEnabled = publicConfig?.passwordLoginEnabled !== false
+ // Redirect-only mode: when OIDC is the only login transport, skip the
+ // chooser and go straight to the provider. Suppressed while the session
+ // probe is in flight (an already-signed-in visitor goes to their
+ // destination instead of the IdP), in WxWork-only environments, and when
+ // the provider bounced back with ?oidcError= - otherwise this would loop.
+ const shouldRedirectToOIDC = Boolean(
+ ready &&
+ !session &&
+ publicConfig &&
+ publicConfig.oidcEnabled &&
+ !publicConfig.passwordLoginEnabled &&
+ !publicConfig.wxworkEnabled &&
+ !oidcError,
+ )
+
+ useEffect(() => {
+ if (!shouldRedirectToOIDC) {
+ return
+ }
+ window.location.href = `/api/auth/oidc_login?next=${encodeURIComponent(redirectPath)}`
+ }, [shouldRedirectToOIDC, redirectPath])
+
useEffect(() => {
if (session) {
router.replace(redirectPath)
@@ -151,6 +173,19 @@ export function LoginForm({
)
}
+ if (shouldRedirectToOIDC) {
+ return (
+
+
+
+
+
{t("auth.redirectingToOidc")}
+
+
+
+ )
+ }
+
return (
Date: Tue, 22 Sep 2026 23:40:07 +0700
Subject: [PATCH 8/8] fix(sync): dedupe the duplicated Slack handler test and
restore upstream coverage
The merge left TestSlackWebhook_Handler declared twice (the fork's
whatsapp_slack_handler_test.go and upstream's slack_handler_test.go).
Drop the fork's copy - upstream's version is the newer signed-payload
form and already covers the unsigned-rejection case - and keep the
WhatsApp test in the fork file. Also restore the upstream-only tests
the merge had dropped (slack inbound rejection/replay/challenge,
discord guild-scope and wrong-secret, OIDC first-login lowest role)
and the slack/discord setup i18n keys the channel dialog renders.
---
internal/handlers/third/slack_handler_test.go | 4 +
.../third/whatsapp_slack_handler_test.go | 139 ------------------
2 files changed, 4 insertions(+), 139 deletions(-)
diff --git a/internal/handlers/third/slack_handler_test.go b/internal/handlers/third/slack_handler_test.go
index 9e7b82e2..72bf72a9 100644
--- a/internal/handlers/third/slack_handler_test.go
+++ b/internal/handlers/third/slack_handler_test.go
@@ -150,4 +150,8 @@ func TestSlackWebhook_Handler(t *testing.T) {
if identity == nil {
t.Fatalf("expected customer identity for U_USER_777")
}
+
+ if identity == nil {
+ t.Fatalf("expected customer identity for U_USER_777")
+ }
}
diff --git a/internal/handlers/third/whatsapp_slack_handler_test.go b/internal/handlers/third/whatsapp_slack_handler_test.go
index 73dbb8ff..d28f20a9 100644
--- a/internal/handlers/third/whatsapp_slack_handler_test.go
+++ b/internal/handlers/third/whatsapp_slack_handler_test.go
@@ -8,7 +8,6 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
- "strconv"
"testing"
"time"
@@ -33,19 +32,6 @@ func signWhatsAppTestPayload(payload []byte) string {
return "sha256=" + hex.EncodeToString(mac.Sum(nil))
}
-// slackTestSigningSecret is the Slack signing secret the test channel is
-// configured with.
-const slackTestSigningSecret = "test_signing_secret"
-
-// signSlackTestPayload builds the X-Slack-Request-Timestamp and
-// X-Slack-Signature headers Slack would send for this body right now.
-func signSlackTestPayload(payload []byte) (string, string) {
- timestamp := strconv.FormatInt(time.Now().Unix(), 10)
- mac := hmac.New(sha256.New, []byte(slackTestSigningSecret))
- mac.Write([]byte("v0:" + timestamp + ":" + string(payload)))
- return timestamp, "v0=" + hex.EncodeToString(mac.Sum(nil))
-}
-
func TestWhatsAppWebhook_Handler(t *testing.T) {
gin.SetMode(gin.TestMode)
db := setupThirdHandlerTestDB(t)
@@ -171,128 +157,3 @@ func TestWhatsAppWebhook_Handler(t *testing.T) {
t.Fatalf("expected customer identity for 1234567890")
}
}
-
-func TestSlackWebhook_Handler(t *testing.T) {
- gin.SetMode(gin.TestMode)
- db := setupThirdHandlerTestDB(t)
-
- now := time.Now()
- agent := &models.AIAgent{
- Name: "Slack Agent",
- ServiceMode: enums.IMConversationServiceModeAIFirst,
- PublishedRevisionID: 1,
- WelcomeMessage: "Hello Slack User!",
- Status: enums.StatusOk,
- AuditFields: models.AuditFields{CreatedAt: now, UpdatedAt: now},
- }
- _ = db.Create(agent)
-
- slackConfig, _ := json.Marshal(dto.SlackChannelConfig{
- BotToken: "xoxb-test-token",
- SigningSecret: slackTestSigningSecret,
- TeamID: "T_SLACK_100",
- DefaultChannel: "C_GENERAL",
- })
-
- operator := &dto.AuthPrincipal{UserID: 1, Username: "admin"}
- channel, err := services.ChannelService.CreateChannel(request.CreateChannelRequest{
- Name: "Slack Channel",
- ChannelType: enums.ChannelTypeSlack,
- AIAgentID: agent.ID,
- AIAgentRolloutPercent: 100,
- ConfigJSON: string(slackConfig),
- Status: int(enums.StatusOk),
- }, operator)
- if err != nil {
- t.Fatalf("CreateChannel failed: %v", err)
- }
-
- router := gin.New()
- router.POST("/api/third/slack/webhook/:channel_id", SlackPostWebhook)
- router.POST("/api/third/slack/webhook", SlackPostWebhook)
-
- // 1. URL Verification
- challengePayload := []byte(`{
- "token": "token123",
- "challenge": "slack_challenge_string_999",
- "type": "url_verification"
- }`)
- reqChallenge, _ := http.NewRequest(http.MethodPost, "/api/third/slack/webhook/"+channel.ChannelID, bytes.NewBuffer(challengePayload))
- reqChallenge.Header.Set("Content-Type", "application/json")
- recChallenge := httptest.NewRecorder()
- router.ServeHTTP(recChallenge, reqChallenge)
-
- if recChallenge.Code != http.StatusOK {
- t.Fatalf("expected 200 OK for challenge, got: %d", recChallenge.Code)
- }
- var challengeResp map[string]any
- _ = json.Unmarshal(recChallenge.Body.Bytes(), &challengeResp)
- if challengeResp["challenge"] != "slack_challenge_string_999" {
- t.Fatalf("expected challenge in body, got: %+v", challengeResp)
- }
-
- // 2. Event Callback
- eventPayload := []byte(`{
- "token": "token123",
- "team_id": "T_SLACK_100",
- "type": "event_callback",
- "event": {
- "type": "message",
- "user": "U_USER_777",
- "text": "Hello support team on Slack!",
- "ts": "1725260000.000100",
- "channel": "C_GENERAL"
- }
- }`)
- reqEvent, _ := http.NewRequest(http.MethodPost, "/api/third/slack/webhook/"+channel.ChannelID, bytes.NewBuffer(eventPayload))
- reqEvent.Header.Set("Content-Type", "application/json")
- slackTimestamp, slackSignature := signSlackTestPayload(eventPayload)
- reqEvent.Header.Set("X-Slack-Request-Timestamp", slackTimestamp)
- reqEvent.Header.Set("X-Slack-Signature", slackSignature)
- recEvent := httptest.NewRecorder()
- router.ServeHTTP(recEvent, reqEvent)
-
- if recEvent.Code != http.StatusOK {
- t.Fatalf("expected 200 OK for event, got: %d", recEvent.Code)
- }
-
- // Verify identity
- identity := repositories.CustomerIdentityRepository.FindOne(db, sqls.NewCnd().
- Eq("external_source", enums.ExternalSourceSlack).
- Eq("external_id", "U_USER_777"))
- if identity == nil {
- t.Fatalf("expected customer identity for U_USER_777")
- }
-
- // 3. Unsigned delivery is rejected: once a signing secret resolves for the
- // channel, a payload without Slack signature headers must not provision
- // anything. The handler still answers 200 ok=false so Slack does not retry.
- unsignedPayload := []byte(`{
- "token": "token123",
- "team_id": "T_SLACK_100",
- "type": "event_callback",
- "event": {
- "type": "message",
- "user": "U_USER_888",
- "text": "Unsigned spoof attempt",
- "ts": "1725260001.000100",
- "channel": "C_GENERAL"
- }
- }`)
- reqUnsigned, _ := http.NewRequest(http.MethodPost, "/api/third/slack/webhook/"+channel.ChannelID, bytes.NewBuffer(unsignedPayload))
- reqUnsigned.Header.Set("Content-Type", "application/json")
- recUnsigned := httptest.NewRecorder()
- router.ServeHTTP(recUnsigned, reqUnsigned)
-
- var unsignedResp map[string]any
- _ = json.Unmarshal(recUnsigned.Body.Bytes(), &unsignedResp)
- if unsignedResp["ok"] != false {
- t.Fatalf("expected unsigned delivery to be rejected with ok=false, got: %+v", unsignedResp)
- }
- unsignedIdentity := repositories.CustomerIdentityRepository.FindOne(db, sqls.NewCnd().
- Eq("external_source", enums.ExternalSourceSlack).
- Eq("external_id", "U_USER_888"))
- if unsignedIdentity != nil {
- t.Fatalf("unsigned delivery must not provision an identity for U_USER_888")
- }
-}