From d6801f9de2f2dd5a73dea4fa2837dfeb83d012fb Mon Sep 17 00:00:00 2001 From: zhx Date: Wed, 15 Jul 2026 16:40:00 +0800 Subject: [PATCH] feat(admin): infer external tenant from account --- server/admin/docs/外管后台.md | 12 +- .../externaladmin/global_username_test.go | 115 ++++++++++++++++++ .../internal/modules/externaladmin/handler.go | 14 ++- .../modules/externaladmin/rate_limit_test.go | 24 ++-- .../internal/modules/externaladmin/service.go | 114 +++++++++++++---- .../externaladmin/service_login_test.go | 98 ++++++++++++++- .../internal/modules/externaladmin/types.go | 1 - .../100_external_admin_global_username.sql | 13 ++ 8 files changed, 343 insertions(+), 48 deletions(-) create mode 100644 server/admin/internal/modules/externaladmin/global_username_test.go create mode 100644 server/admin/migrations/100_external_admin_global_username.sql diff --git a/server/admin/docs/外管后台.md b/server/admin/docs/外管后台.md index 3510d6f9..3efc8d41 100644 --- a/server/admin/docs/外管后台.md +++ b/server/admin/docs/外管后台.md @@ -7,7 +7,7 @@ - 主后台 `/operations/external-admin-users` 继续使用主后台 JWT、RBAC 和全局 App 选择器，只负责查找 App 用户、签发外管账号、启停账号和重置密码。 - 外管入口 `/external-admin/` 使用独立账号、服务端 opaque session、CSRF token 和首次改密流程，不读取主后台 token 或 localStorage。 - 外管账号、会话、登录日志和操作日志分别保存在 `external_admin_accounts`、`external_admin_sessions`、`external_admin_login_logs`、`external_admin_operation_logs`。 -- `app_code` 固化在账号和会话中。Fami 账号只能访问 Fami 数据；请求头缺省时由会话补齐，伪造其他 App 时直接拒绝。 +- 登录只提交外管账号和密码，不提交或选择 App。账号名全局唯一，服务端通过账号行自动确定 `app_code` 并固化到会话；Fami 账号只能访问 Fami 数据，请求头缺省时由会话补齐，伪造其他 App 时直接拒绝。 ## 账号签发和岗位边界 @@ -15,7 +15,7 @@ - `platform-admin`：查看、创建、启停和重置密码。创建和重置等同于签发一组固定的高权限外管能力，因此只允许平台管理员执行。 - `ops-admin`、`operations-specialist`：查看、启停。运营岗位不能通过创建或重置账号间接获得自身权限矩阵中没有的 VIP 发放等能力。 -- 同一 App 内，外管账号名和绑定 App 用户均唯一；不同 App 可以使用同名外管账号。 +- 外管登录账号名跨 App 全局唯一；绑定 App 用户仍按 `(app_code, linked_app_user_id)` 唯一。创建服务先做全局账号名检查，数据库唯一索引负责收敛并发创建竞态。 - 创建、重置后的临时密码必须首次修改；停用或重置会撤销该账号的全部活跃会话。 ## 外管能力 @@ -34,14 +34,14 @@ - Session cookie 为 HttpOnly，作用域仅 `/api/v1/external`；写请求同时校验 CSRF cookie、header 和服务端 hash。 - Session 绝对有效期默认 12 小时。每次请求都会确认账号和 App 仍为 active；App 停用后旧会话不能绕过 `/auth/me` 直接调用业务接口。 -- 同一账号连续失败 5 次后锁定 15 分钟；App、账号和密码错误对外使用统一文案。 -- Redis 在 MySQL 查询、bcrypt 和登录日志之前执行 60 秒固定窗口限流：系统全局 300、单 App 120、单 IP 30、单 App+账号 10。四层计数在一次 Lua 中原子完成，拒绝请求仍消耗总闸配额。 +- 同一账号连续失败 5 次后锁定 15 分钟；账号、密码或账号所属 App 不可用均对外返回相同的 HTTP 401 / 业务码 40100，前端按当前语言渲染本地化文案。 +- Redis 在账号查询、bcrypt 和登录日志之前，先按系统全局 300、单真实 IP 30、全局账号名 10 执行 60 秒固定窗口限流；命中唯一账号后，再按服务端解析出的 App 执行单 App 120 限流。两个阶段不会重复消耗系统/IP/账号计数，请求中的旧版 `appCode` 字段即使存在也会被忽略。 - 登录请求体上限为 8 KiB；限流 Redis 在 200 ms 内不可用时 fail-closed 返回 503，不降级为可绕过的单机计数。 - 认证接口统一返回 `Cache-Control: no-store`。 ## 上线步骤 -1. 通过 admin-server 迁移器依次应用 `migrations/098_external_admin_portal.sql` 和 `migrations/099_external_admin_credential_delegation.sql`。098 只新建四张认证/审计表并按唯一索引写入菜单和权限元数据；099 通过角色、权限唯一索引和关联表主键定点撤销旧版 098 可能误授予运营岗位的凭据权限。两者都不扫描用户、房间、钱包等大业务表；不要在高峰期临时改写迁移内容。 +1. 通过 admin-server 迁移器依次应用 `migrations/098_external_admin_portal.sql`、`099_external_admin_credential_delegation.sql` 和 `100_external_admin_global_username.sql`。发布前先执行 `EXPLAIN SELECT username, COUNT(*) FROM external_admin_accounts GROUP BY username HAVING COUNT(*) > 1` 评估访问计划，再执行同一查询确认结果为空。该检查和 100 的唯一索引构建只扫描小型凭据表，不访问用户、房间或钱包表；100 使用 `INPLACE + LOCK=NONE`，但仍建议避开凭据集中创建时段。若存在重复，DDL 会 fail-closed，不会自动改名或删除账号；先由业务确认保留的登录名并处理冲突，再清理迁移器 dirty 记录并重跑。 2. 确认 Redis 已启用且 `/readyz` 为成功。生产配置缺少 Redis 时 admin-server 会拒绝启动，运行中限流 Redis 故障会阻断新登录但不破坏已建立会话。 3. 配置 `trusted_proxies` 为真实入口链路的精确 IP/CIDR。默认只信任 loopback；不要信任 `0.0.0.0/0`、`::/0` 或整段未知私网。 4. 构建并发布前端 `dist/external-admin/`，保留 Nginx 对 `/external-admin` 的 301 和所有 `/external-admin/*` 深链回退到独立 HTML。 @@ -49,7 +49,7 @@ ## 上线验收 -- Fami 创建的账号登录后请求 Lalu 的 `X-App-Code` 必须返回 403，省略 header 时仍只能看到 Fami 数据。 +- 登录请求不带 App 即可进入账号所属租户；即使旧客户端伪造其他 `appCode`，也必须登录到账号行所属 App。Fami 账号登录后请求 Lalu 的 `X-App-Code` 必须返回 403，省略 header 时仍只能看到 Fami 数据。 - 外管账号或 App 停用后，已有会话直接访问任一业务接口必须返回 401。 - 临时密码登录后，除会话、退出和修改密码外的业务接口必须返回 403；改密后才可进入授权页面。 - 从同一真实 IP 连发超过阈值的登录请求必须返回 429 和 `Retry-After`；伪造 `X-Forwarded-For` 不能改变限流身份。 diff --git a/server/admin/internal/modules/externaladmin/global_username_test.go b/server/admin/internal/modules/externaladmin/global_username_test.go new file mode 100644 index 00000000..09d3b04a --- /dev/null +++ b/server/admin/internal/modules/externaladmin/global_username_test.go @@ -0,0 +1,115 @@ +package externaladmin + +import ( + "errors" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/gin-gonic/gin" + "gorm.io/driver/mysql" + "gorm.io/gorm" +) + +func TestCreateAccountRejectsUsernameAlreadyUsedByAnotherApp(t *testing.T) { + adminSQL, adminMock, err := sqlmock.New() + if err != nil { + t.Fatalf("create admin sql mock: %v", err) + } + defer adminSQL.Close() + adminDB, err := gorm.Open(mysql.New(mysql.Config{Conn: adminSQL, SkipInitializeWithVersion: true}), &gorm.Config{}) + if err != nil { + t.Fatalf("create admin gorm: %v", err) + } + userDB, userMock, err := sqlmock.New() + if err != nil { + t.Fatalf("create user sql mock: %v", err) + } + defer userDB.Close() + + // The check intentionally has no app_code predicate. It avoids owner-DB and + // bcrypt work for an obvious conflict; migration 100's unique index closes the + // concurrent-create race after this advisory check. + adminMock.ExpectQuery("SELECT `id` FROM `external_admin_accounts` WHERE username = \\? LIMIT \\?"). + WithArgs("shared-operator", 1). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(9)) + service := NewService(adminDB, userDB, Config{}) + _, err = service.CreateAccount(t.Context(), CreateInput{ + AppCode: "lalu", TargetUserID: "8888", Username: "Shared-Operator", + Password: "temporary-password", CreatedByAdminID: 1, + }) + if !errors.Is(err, ErrAccountConflict) { + t.Fatalf("create error = %v, want global username conflict", err) + } + if err := adminMock.ExpectationsWereMet(); err != nil { + t.Fatalf("admin sql expectations: %v", err) + } + if err := userMock.ExpectationsWereMet(); err != nil { + t.Fatalf("user DB must not be queried after global conflict: %v", err) + } +} + +func TestLoginIgnoresLegacyRequestAppAndAuditsUnknownAccountWithoutTenant(t *testing.T) { + adminSQL, adminMock, err := sqlmock.New() + if err != nil { + t.Fatalf("create admin sql mock: %v", err) + } + defer adminSQL.Close() + adminDB, err := gorm.Open(mysql.New(mysql.Config{Conn: adminSQL, SkipInitializeWithVersion: true}), &gorm.Config{}) + if err != nil { + t.Fatalf("create admin gorm: %v", err) + } + + adminMock.ExpectQuery("SELECT `id`,`app_code` FROM `external_admin_accounts` WHERE username = \\? LIMIT \\?"). + WithArgs("missing-operator", 1). + WillReturnRows(sqlmock.NewRows([]string{"id", "app_code"})) + // Unknown names use an empty app_code sentinel. The legacy request value "lalu" + // must not be copied into security audit data or used to select a tenant. + adminMock.ExpectBegin() + adminMock.ExpectExec("INSERT INTO `external_admin_login_logs`"). + WithArgs(nil, "", "missing-operator", "192.0.2.10", "", "failed", "invalid_credentials", sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(1, 1)) + adminMock.ExpectCommit() + + gin.SetMode(gin.TestMode) + engine := gin.New() + handler := New(adminDB, nil, Config{}, nil, WithLoginRateLimiter(allowFixedWindowLimiter{})) + RegisterExternalRoutes(engine.Group("/api/v1"), handler, BusinessHandlers{}) + request := httptest.NewRequest(http.MethodPost, "/api/v1/external/auth/login", strings.NewReader(`{"appCode":"lalu","account":"missing-operator","password":"wrong-password"}`)) + request.Header.Set("Content-Type", "application/json") + request.RemoteAddr = "192.0.2.10:12345" + responseRecorder := httptest.NewRecorder() + engine.ServeHTTP(responseRecorder, request) + if responseRecorder.Code != http.StatusUnauthorized { + t.Fatalf("status=%d body=%s", responseRecorder.Code, responseRecorder.Body.String()) + } + if !strings.Contains(responseRecorder.Body.String(), `"code":40100`) || strings.Contains(responseRecorder.Body.String(), "App") { + t.Fatalf("login response must expose only stable auth code and generic text: %s", responseRecorder.Body.String()) + } + if err := adminMock.ExpectationsWereMet(); err != nil { + t.Fatalf("admin sql expectations: %v", err) + } +} + +func TestGlobalUsernameMigrationIsOnlineAndFailClosed(t *testing.T) { + body, err := os.ReadFile("../../../migrations/100_external_admin_global_username.sql") + if err != nil { + t.Fatalf("read migration 100: %v", err) + } + sqlText := string(body) + for _, token := range []string{ + "ADD UNIQUE KEY uk_external_admin_accounts_username (username)", + "DROP INDEX uk_external_admin_accounts_app_username", + "ALGORITHM=INPLACE", "LOCK=NONE", + } { + if !strings.Contains(sqlText, token) { + t.Fatalf("migration 100 missing %q", token) + } + } + if strings.Contains(strings.ToUpper(sqlText), "UPDATE EXTERNAL_ADMIN_ACCOUNTS") || strings.Contains(strings.ToUpper(sqlText), "DELETE FROM EXTERNAL_ADMIN_ACCOUNTS") { + t.Fatal("migration must not silently rename or delete duplicate credentials") + } +} diff --git a/server/admin/internal/modules/externaladmin/handler.go b/server/admin/internal/modules/externaladmin/handler.go index 594190c4..700c9c79 100644 --- a/server/admin/internal/modules/externaladmin/handler.go +++ b/server/admin/internal/modules/externaladmin/handler.go @@ -76,7 +76,6 @@ type passwordRequest struct { } type loginRequest struct { - AppCode string `json:"appCode"` Account string `json:"account"` Username string `json:"username"` Password string `json:"password"` @@ -143,7 +142,7 @@ func (h *Handler) CreateAccount(c *gin.Context) { Username: request.Username, Password: request.Password, CreatedByAdminID: middleware.CurrentUserID(c), }) if errors.Is(err, ErrAccountConflict) { - response.Conflict(c, "当前 App 的账户名称或绑定用户已存在") + response.Conflict(c, "外管账户名称已被使用，或当前 App 的绑定用户已存在") return } if errors.Is(err, ErrTargetNotFound) { @@ -253,7 +252,7 @@ func (h *Handler) Login(c *gin.Context) { username = request.Username } result, err := h.service.Login(c.Request.Context(), LoginInput{ - AppCode: request.AppCode, Username: username, Password: request.Password, + Username: username, Password: request.Password, IP: c.ClientIP(), UserAgent: c.Request.UserAgent(), }) var rateLimitErr *LoginRateLimitError @@ -271,13 +270,20 @@ func (h *Handler) Login(c *gin.Context) { return } if errors.Is(err, ErrInvalidCredentials) { - response.Unauthorized(c, "App、账号或密码错误") + // App is deliberately absent from both input and error text. The account row is + // the only tenant authority, while the stable 401/40100 pair lets every locale + // render its own generic message without parsing this server-side fallback. + response.Unauthorized(c, "账号或密码错误") return } if err != nil { response.ServerError(c, "登录服务暂不可用") return } + // The global App middleware cannot know the tenant before authentication and + // therefore writes its generic default header. Replace it with the App resolved + // from the credential row so the successful response has one consistent tenant. + c.Header(appctx.HeaderAppCode, result.View.AppCode) h.setSessionCookies(c, result.SessionToken, result.CSRFToken) c.Header(CSRFHeaderName, result.CSRFToken) response.OK(c, result.View) diff --git a/server/admin/internal/modules/externaladmin/rate_limit_test.go b/server/admin/internal/modules/externaladmin/rate_limit_test.go index 941a38e4..5c6aa719 100644 --- a/server/admin/internal/modules/externaladmin/rate_limit_test.go +++ b/server/admin/internal/modules/externaladmin/rate_limit_test.go @@ -27,6 +27,7 @@ type recordingFixedWindowLimiter struct { calls int window time.Duration rules []cache.FixedWindowRule + batches [][]cache.FixedWindowRule deadlineSet bool deadlineIn time.Duration } @@ -35,6 +36,7 @@ func (limiter *recordingFixedWindowLimiter) ConsumeFixedWindow(ctx context.Conte limiter.calls++ limiter.window = window limiter.rules = append([]cache.FixedWindowRule(nil), rules...) + limiter.batches = append(limiter.batches, append([]cache.FixedWindowRule(nil), rules...)) deadline, ok := ctx.Deadline() limiter.deadlineSet = ok if ok { @@ -43,7 +45,7 @@ func (limiter *recordingFixedWindowLimiter) ConsumeFixedWindow(ctx context.Conte return limiter.decision, limiter.err } -func TestLoginRateLimiterUsesFourHashedClusterSafeLayersBeforeDatabase(t *testing.T) { +func TestLoginRateLimiterUsesTrustedHashedLayersBeforeAccountResolution(t *testing.T) { limiter := &recordingFixedWindowLimiter{decision: cache.FixedWindowDecision{Allowed: true}} service := NewService(&gorm.DB{}, nil, Config{LoginRateLimit: LoginRateLimitConfig{ Window: time.Minute, SystemLimit: 300, AppLimit: 120, IPLimit: 30, IdentityLimit: 10, @@ -52,17 +54,17 @@ func TestLoginRateLimiterUsesFourHashedClusterSafeLayersBeforeDatabase(t *testin // Empty password is invalid, but this syntactically valid attempt must consume // distributed counters before dummy bcrypt/input rejection. - _, err := service.Login(t.Context(), LoginInput{AppCode: "FAMI", Username: "Operator", IP: "203.0.113.44"}) + _, err := service.Login(t.Context(), LoginInput{Username: "Operator", IP: "203.0.113.44"}) if !errors.Is(err, ErrInvalidCredentials) { t.Fatalf("login error = %v", err) } - if limiter.calls != 1 || limiter.window != time.Minute || len(limiter.rules) != 4 { + if limiter.calls != 1 || limiter.window != time.Minute || len(limiter.rules) != 3 { t.Fatalf("limiter call=%d window=%s rules=%+v", limiter.calls, limiter.window, limiter.rules) } if !limiter.deadlineSet || limiter.deadlineIn <= 0 || limiter.deadlineIn > loginLimiterTimeout+25*time.Millisecond { t.Fatalf("independent limiter deadline = %s set=%t", limiter.deadlineIn, limiter.deadlineSet) } - wantLimits := []uint64{300, 120, 30, 10} + wantLimits := []uint64{300, 30, 10} for index, rule := range limiter.rules { if rule.Limit != wantLimits[index] { t.Fatalf("rule %d limit=%d", index, rule.Limit) @@ -70,7 +72,7 @@ func TestLoginRateLimiterUsesFourHashedClusterSafeLayersBeforeDatabase(t *testin if !strings.Contains(rule.Key, loginLimiterClusterTag) { t.Fatalf("rule %d is not Redis Cluster-safe: %s", index, rule.Key) } - for _, secret := range []string{"fami", "operator", "203.0.113.44"} { + for _, secret := range []string{"operator", "203.0.113.44"} { if strings.Contains(rule.Key, secret) { t.Fatalf("rule %d leaks %q: %s", index, secret, rule.Key) } @@ -89,7 +91,7 @@ func TestLoginRateLimitDenialAndRedisFailurePrecedeAllDatabaseWork(t *testing.T) limiter := &recordingFixedWindowLimiter{decision: cache.FixedWindowDecision{Allowed: false, RetryAfter: 1500 * time.Millisecond}} service := NewService(nil, nil, Config{}) service.limiter = limiter - _, err := service.Login(t.Context(), LoginInput{AppCode: "fami", Username: "operator", Password: "password", IP: "127.0.0.1"}) + _, err := service.Login(t.Context(), LoginInput{Username: "operator", Password: "password", IP: "127.0.0.1"}) var rateErr *LoginRateLimitError if !errors.As(err, &rateErr) || rateErr.RetryAfter != 1500*time.Millisecond { t.Fatalf("rate error = %#v", err) @@ -100,7 +102,7 @@ func TestLoginRateLimitDenialAndRedisFailurePrecedeAllDatabaseWork(t *testing.T) limiter := &recordingFixedWindowLimiter{err: errors.New("redis down")} service := NewService(nil, nil, Config{}) service.limiter = limiter - _, err := service.Login(t.Context(), LoginInput{AppCode: "fami", Username: "operator", Password: "password", IP: "127.0.0.1"}) + _, err := service.Login(t.Context(), LoginInput{Username: "operator", Password: "password", IP: "127.0.0.1"}) if !errors.Is(err, ErrLoginLimiterFailed) { t.Fatalf("login error = %v", err) } @@ -108,7 +110,7 @@ func TestLoginRateLimitDenialAndRedisFailurePrecedeAllDatabaseWork(t *testing.T) t.Run("missing redis", func(t *testing.T) { service := NewService(nil, nil, Config{}) - _, err := service.Login(t.Context(), LoginInput{AppCode: "fami", Username: "operator", Password: "password", IP: "127.0.0.1"}) + _, err := service.Login(t.Context(), LoginInput{Username: "operator", Password: "password", IP: "127.0.0.1"}) if !errors.Is(err, ErrLoginLimiterFailed) { t.Fatalf("login error = %v", err) } @@ -125,7 +127,7 @@ func TestLoginHandlerReturns429RetryAfterAnd503OnLimiterFailure(t *testing.T) { }{ { name: "limited", limiter: &recordingFixedWindowLimiter{decision: cache.FixedWindowDecision{Allowed: false, RetryAfter: 1500 * time.Millisecond}}, - wantStatus: http.StatusTooManyRequests, wantRetry: "2", body: `{"appCode":"fami","account":"operator","password":"password"}`, + wantStatus: http.StatusTooManyRequests, wantRetry: "2", body: `{"account":"operator","password":"password"}`, }, { name: "valid empty json still counted", limiter: &recordingFixedWindowLimiter{decision: cache.FixedWindowDecision{Allowed: false, RetryAfter: time.Second}}, @@ -133,7 +135,7 @@ func TestLoginHandlerReturns429RetryAfterAnd503OnLimiterFailure(t *testing.T) { }, { name: "redis unavailable", limiter: &recordingFixedWindowLimiter{err: errors.New("redis down")}, - wantStatus: http.StatusServiceUnavailable, body: `{"appCode":"fami","account":"operator","password":"password"}`, + wantStatus: http.StatusServiceUnavailable, body: `{"account":"operator","password":"password"}`, }, } { t.Run(testCase.name, func(t *testing.T) { @@ -164,7 +166,7 @@ func TestLoginHandlerCapsBodyBeforeDistributedLimiter(t *testing.T) { engine := gin.New() handler := New(nil, nil, Config{}, nil, WithLoginRateLimiter(limiter)) RegisterExternalRoutes(engine.Group("/api/v1"), handler, BusinessHandlers{}) - body := `{"appCode":"fami","account":"operator","password":"` + strings.Repeat("x", int(maxLoginBodyBytes)) + `"}` + body := `{"account":"operator","password":"` + strings.Repeat("x", int(maxLoginBodyBytes)) + `"}` request := httptest.NewRequest(http.MethodPost, "/api/v1/external/auth/login", strings.NewReader(body)) request.Header.Set("Content-Type", "application/json") responseRecorder := httptest.NewRecorder() diff --git a/server/admin/internal/modules/externaladmin/service.go b/server/admin/internal/modules/externaladmin/service.go index 0ef51b43..bdc5ba10 100644 --- a/server/admin/internal/modules/externaladmin/service.go +++ b/server/admin/internal/modules/externaladmin/service.go @@ -193,6 +193,17 @@ func (s *Service) CreateAccount(ctx context.Context, input CreateInput) (Account if err := validatePassword(input.Password); err != nil { return AccountDTO{}, err } + // Login has no client-selected App. Check the normalized username globally before + // doing an owner-DB lookup or bcrypt work; the global unique index remains the + // authoritative race-safe guard if two administrators create it concurrently. + var existing model.ExternalAdminAccount + err := s.db.WithContext(ctx).Select("id").Where("username = ?", input.Username).Take(&existing).Error + if err == nil { + return AccountDTO{}, ErrAccountConflict + } + if !errors.Is(err, gorm.ErrRecordNotFound) { + return AccountDTO{}, err + } if _, err := s.findActiveApp(ctx, input.AppCode); err != nil { if errors.Is(err, sql.ErrNoRows) { return AccountDTO{}, ErrInvalidInput @@ -308,29 +319,54 @@ func (s *Service) ResetPassword(ctx context.Context, appCode string, id uint64, } func (s *Service) Login(ctx context.Context, input LoginInput) (LoginResult, error) { - input.AppCode = normalizeAppCode(input.AppCode) input.Username = normalizeUsername(input.Username) input.IP = truncateText(strings.TrimSpace(input.IP), 64) input.UserAgent = truncateText(strings.TrimSpace(input.UserAgent), 512) - // Rate limiting runs before any MySQL lookup, login-log write or bcrypt work. - // This includes syntactically valid attempts with missing/invalid fields, so - // attackers cannot bypass distributed counters by varying malformed identities. - if err := s.consumeLoginRateLimit(ctx, input); err != nil { + // The pre-resolution limiter uses only facts the server can trust: system, real + // client IP and the globally normalized username. AppCode is intentionally absent, + // so a forged tenant cannot select another account or evade any of these counters. + if err := s.consumeUnresolvedLoginRateLimit(ctx, input); err != nil { return LoginResult{}, err } if s.db == nil { return LoginResult{}, errors.New("admin mysql is not configured") } - if !validAppCode(input.AppCode) || !validUsername(input.Username) || input.Password == "" { + if !validUsername(input.Username) || input.Password == "" { security.CheckPassword(s.dummyHash, input.Password) return LoginResult{}, ErrInvalidCredentials } - app, err := s.findActiveApp(ctx, input.AppCode) + + // The global unique index makes this indexed projection the sole tenant resolver. + // Do not accept a request/header App here: only the AppCode stored with the account + // may select owner data, create the session or populate audit rows. + var resolvedAccount model.ExternalAdminAccount + err := s.db.WithContext(ctx).Select("id", "app_code").Where("username = ?", input.Username).Take(&resolvedAccount).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + security.CheckPassword(s.dummyHash, input.Password) + s.writeUnknownLoginLog(ctx, input, "invalid_credentials") + return LoginResult{}, ErrInvalidCredentials + } if err != nil { - // Keep unknown/disabled App, unknown account and wrong password indistinguishable. + return LoginResult{}, err + } + resolvedAccount.AppCode = normalizeAppCode(resolvedAccount.AppCode) + if !validAppCode(resolvedAccount.AppCode) { + security.CheckPassword(s.dummyHash, input.Password) + s.writeResolvedLoginLog(ctx, resolvedAccount.ID, resolvedAccount.AppCode, input, "invalid_account_app") + return LoginResult{}, ErrInvalidCredentials + } + // App quota is consumed in a second Redis call only after the indexed account + // resolution. System/IP/username rules are not repeated, so one attempt increments + // each layer exactly once while still containing a targeted tenant attack. + if err := s.consumeResolvedAppLoginRateLimit(ctx, resolvedAccount.AppCode); err != nil { + return LoginResult{}, err + } + app, err := s.findActiveApp(ctx, resolvedAccount.AppCode) + if err != nil { + // Keep disabled App, unknown account and wrong password indistinguishable. security.CheckPassword(s.dummyHash, input.Password) if errors.Is(err, sql.ErrNoRows) { - s.writeUnknownLoginLog(ctx, input, "invalid_credentials") + s.writeResolvedLoginLog(ctx, resolvedAccount.ID, resolvedAccount.AppCode, input, "app_inactive") return LoginResult{}, ErrInvalidCredentials } return LoginResult{}, err @@ -345,12 +381,12 @@ func (s *Service) Login(ctx context.Context, input LoginInput) (LoginResult, err // Lock the account row while checking/updating the failure counter. Concurrent bad passwords // must serialize on one counter; otherwise attackers could race requests below the lock threshold. err = s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { - err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Where("app_code = ? AND username = ?", input.AppCode, input.Username).First(&account).Error + err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Where("id = ? AND username = ?", resolvedAccount.ID, input.Username).First(&account).Error if errors.Is(err, gorm.ErrRecordNotFound) { security.CheckPassword(s.dummyHash, input.Password) authErr = ErrInvalidCredentials return tx.Create(&model.ExternalAdminLoginLog{ - AppCode: input.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, + AppCode: resolvedAccount.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: "invalid_credentials", }).Error } @@ -360,13 +396,22 @@ func (s *Service) Login(ctx context.Context, input LoginInput) (LoginResult, err accountID := account.ID validPassword := security.CheckPassword(account.PasswordHash, input.Password) + accountAppCode := normalizeAppCode(account.AppCode) + if accountAppCode != resolvedAccount.AppCode { + // AppCode is not mutable through this module, but a concurrent/manual data + // correction between projection and row lock must not create a session whose + // App metadata and tenant scope came from different rows/snapshots. + authErr = ErrInvalidCredentials + return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: accountAppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: "account_app_changed"}).Error + } + account.AppCode = accountAppCode if account.Status != model.ExternalAdminStatusActive { authErr = ErrInvalidCredentials - return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: input.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: "account_disabled"}).Error + return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: account.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: "account_disabled"}).Error } if account.LockedUntilMS > nowMS { authErr = ErrInvalidCredentials - return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: input.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: "account_locked"}).Error + return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: account.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: "account_locked"}).Error } if !validPassword { failedCount := account.FailedLoginCount @@ -385,7 +430,7 @@ func (s *Service) Login(ctx context.Context, input LoginInput) (LoginResult, err return err } authErr = ErrInvalidCredentials - return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: input.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: message}).Error + return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: account.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: message}).Error } var tokenHash string @@ -409,7 +454,7 @@ func (s *Service) Login(ctx context.Context, input LoginInput) (LoginResult, err return err } account.LastLoginAtMS = &nowMS - return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: input.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "success", Message: "login_success"}).Error + return tx.Create(&model.ExternalAdminLoginLog{AccountID: &accountID, AppCode: account.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "success", Message: "login_success"}).Error }) if err != nil { return LoginResult{}, err @@ -425,21 +470,34 @@ func (s *Service) Login(ctx context.Context, input LoginInput) (LoginResult, err return LoginResult{SessionToken: sessionToken, CSRFToken: csrfToken, View: view}, nil } -func (s *Service) consumeLoginRateLimit(ctx context.Context, input LoginInput) error { +func (s *Service) consumeUnresolvedLoginRateLimit(ctx context.Context, input LoginInput) error { if s.limiter == nil { return ErrLoginLimiterFailed } - appDigest := loginRateLimitDigest(input.AppCode) ipDigest := loginRateLimitDigest(input.IP) - identityDigest := loginRateLimitDigest(input.AppCode + "\x00" + input.Username) + identityDigest := loginRateLimitDigest(input.Username) rules := []cache.FixedWindowRule{ - // The system key is intentionally independent of request App. It is the - // non-bypassable circuit breaker across tenant/IP/account rotations. + // These keys are independent of App because no client-provided tenant value + // is trusted before the globally unique username resolves an account. {Key: "rate:" + loginLimiterClusterTag + ":system", Limit: s.cfg.LoginRateLimit.SystemLimit}, - {Key: "rate:" + loginLimiterClusterTag + ":app:" + appDigest, Limit: s.cfg.LoginRateLimit.AppLimit}, {Key: "rate:" + loginLimiterClusterTag + ":ip:" + ipDigest, Limit: s.cfg.LoginRateLimit.IPLimit}, {Key: "rate:" + loginLimiterClusterTag + ":identity:" + identityDigest, Limit: s.cfg.LoginRateLimit.IdentityLimit}, } + return s.consumeLoginRateLimitRules(ctx, rules) +} + +func (s *Service) consumeResolvedAppLoginRateLimit(ctx context.Context, appCode string) error { + if s.limiter == nil { + return ErrLoginLimiterFailed + } + rules := []cache.FixedWindowRule{{ + Key: "rate:" + loginLimiterClusterTag + ":app:" + loginRateLimitDigest(normalizeAppCode(appCode)), + Limit: s.cfg.LoginRateLimit.AppLimit, + }} + return s.consumeLoginRateLimitRules(ctx, rules) +} + +func (s *Service) consumeLoginRateLimitRules(ctx context.Context, rules []cache.FixedWindowRule) error { limitCtx, cancel := context.WithTimeout(ctx, loginLimiterTimeout) defer cancel() decision, err := s.limiter.ConsumeFixedWindow(limitCtx, s.cfg.LoginRateLimit.Window, rules) @@ -691,12 +749,24 @@ func (s *Service) writeUnknownLoginLog(ctx context.Context, input LoginInput, me if s.db == nil { return } + // Empty app_code is the audit sentinel for an unresolved username. Persisting a + // request-supplied tenant here would incorrectly attribute an attack to a real App. _ = s.db.WithContext(ctx).Create(&model.ExternalAdminLoginLog{ - AppCode: input.AppCode, Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, + AppCode: "", Username: input.Username, IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: message, }).Error } +func (s *Service) writeResolvedLoginLog(ctx context.Context, accountID uint64, appCode string, input LoginInput, message string) { + if s.db == nil { + return + } + _ = s.db.WithContext(ctx).Create(&model.ExternalAdminLoginLog{ + AccountID: &accountID, AppCode: normalizeAppCode(appCode), Username: input.Username, + IP: input.IP, UserAgent: input.UserAgent, Status: "failed", Message: message, + }).Error +} + func accountDTO(account model.ExternalAdminAccount, linkedUser LinkedUser) AccountDTO { permissions, _ := decodePermissions(account.PermissionsJSON) if linkedUser.UserID == 0 { diff --git a/server/admin/internal/modules/externaladmin/service_login_test.go b/server/admin/internal/modules/externaladmin/service_login_test.go index 11475dee..561997e4 100644 --- a/server/admin/internal/modules/externaladmin/service_login_test.go +++ b/server/admin/internal/modules/externaladmin/service_login_test.go @@ -3,9 +3,11 @@ package externaladmin import ( "database/sql/driver" "errors" + "strings" "testing" "time" + "hyapp-admin-server/internal/cache" "hyapp-admin-server/internal/security" "github.com/DATA-DOG/go-sqlmock" @@ -33,12 +35,17 @@ func TestLoginFailureLimitPersistsTimedLockAndUnifiedError(t *testing.T) { if err != nil { t.Fatalf("hash fixture password: %v", err) } + // Login resolves the tenant from the globally unique account name before it + // touches the owner App database; no request App participates in either query. + adminMock.ExpectQuery("SELECT `id`,`app_code` FROM `external_admin_accounts` WHERE username = \\? LIMIT \\?"). + WithArgs("operator", 1). + WillReturnRows(sqlmock.NewRows([]string{"id", "app_code"}).AddRow(7, "fami")) userMock.ExpectQuery("SELECT app_code, app_name, logo_url FROM apps"). WithArgs("fami"). WillReturnRows(sqlmock.NewRows([]string{"app_code", "app_name", "logo_url"}).AddRow("fami", "Fami", "logo")) adminMock.ExpectBegin() - adminMock.ExpectQuery("SELECT \\* FROM `external_admin_accounts` WHERE app_code = \\? AND username = \\?"). - WithArgs("fami", "operator", 1). + adminMock.ExpectQuery("SELECT \\* FROM `external_admin_accounts` WHERE id = \\? AND username = \\?"). + WithArgs(uint64(7), "operator", 1). WillReturnRows(sqlmock.NewRows([]string{ "id", "app_code", "linked_app_user_id", "username", "password_hash", "permissions_json", "status", "password_change_required", "failed_login_count", "locked_until_ms", "created_by_admin_id", "created_at_ms", "updated_at_ms", @@ -52,9 +59,10 @@ func TestLoginFailureLimitPersistsTimedLockAndUnifiedError(t *testing.T) { adminMock.ExpectCommit() service := NewService(adminDB, userDB, Config{MaxLoginFailures: 1, LockDuration: 10 * time.Minute}) - service.limiter = allowFixedWindowLimiter{} + limiter := &recordingFixedWindowLimiter{decision: cache.FixedWindowDecision{Allowed: true}} + service.limiter = limiter _, err = service.Login(t.Context(), LoginInput{ - AppCode: "FAMI", Username: "Operator", Password: "wrong-password", IP: "127.0.0.1", UserAgent: "browser", + Username: "Operator", Password: "wrong-password", IP: "127.0.0.1", UserAgent: "browser", }) if !errors.Is(err, ErrInvalidCredentials) { t.Fatalf("login error = %v, want unified invalid credentials", err) @@ -65,6 +73,88 @@ func TestLoginFailureLimitPersistsTimedLockAndUnifiedError(t *testing.T) { if err := userMock.ExpectationsWereMet(); err != nil { t.Fatalf("user sql expectations: %v", err) } + if limiter.calls != 2 || len(limiter.batches) != 2 || len(limiter.batches[0]) != 3 || len(limiter.batches[1]) != 1 { + t.Fatalf("rate-limit batches = %+v", limiter.batches) + } + if !strings.Contains(limiter.batches[1][0].Key, ":app:") || limiter.batches[1][0].Limit != 120 { + t.Fatalf("resolved App limiter = %+v", limiter.batches[1]) + } + for _, rule := range limiter.batches[1] { + if strings.Contains(rule.Key, ":system") || strings.Contains(rule.Key, ":ip:") || strings.Contains(rule.Key, ":identity:") { + t.Fatalf("resolved App phase repeated a pre-resolution counter: %s", rule.Key) + } + } +} + +func TestLoginWithoutAppCreatesSessionFromResolvedAccountTenant(t *testing.T) { + adminSQL, adminMock, err := sqlmock.New() + if err != nil { + t.Fatalf("create admin sql mock: %v", err) + } + defer adminSQL.Close() + adminDB, err := gorm.Open(mysql.New(mysql.Config{Conn: adminSQL, SkipInitializeWithVersion: true}), &gorm.Config{}) + if err != nil { + t.Fatalf("create admin gorm db: %v", err) + } + userDB, userMock, err := sqlmock.New() + if err != nil { + t.Fatalf("create user sql mock: %v", err) + } + defer userDB.Close() + passwordHash, err := security.HashPassword("correct-password") + if err != nil { + t.Fatalf("hash fixture password: %v", err) + } + + adminMock.ExpectQuery("SELECT `id`,`app_code` FROM `external_admin_accounts` WHERE username = \\? LIMIT \\?"). + WithArgs("operator", 1). + WillReturnRows(sqlmock.NewRows([]string{"id", "app_code"}).AddRow(7, "fami")) + userMock.ExpectQuery("SELECT app_code, app_name, logo_url FROM apps"). + WithArgs("fami"). + WillReturnRows(sqlmock.NewRows([]string{"app_code", "app_name", "logo_url"}).AddRow("fami", "Fami", "logo")) + adminMock.ExpectBegin() + adminMock.ExpectQuery("SELECT \\* FROM `external_admin_accounts` WHERE id = \\? AND username = \\?"). + WithArgs(uint64(7), "operator", 1). + WillReturnRows(sqlmock.NewRows([]string{ + "id", "app_code", "linked_app_user_id", "username", "password_hash", "permissions_json", "status", + "password_change_required", "failed_login_count", "locked_until_ms", "last_login_at_ms", "created_by_admin_id", "created_at_ms", "updated_at_ms", + }).AddRow(7, "fami", 9001, "operator", passwordHash, `["bd:view"]`, "active", false, 0, 0, nil, 1, 1, 1)) + // The session insert is the persistence proof: AppCode comes from the account + // projection even though LoginInput has no App field at all. + adminMock.ExpectExec("INSERT INTO `external_admin_sessions`"). + WithArgs(uint64(7), "fami", sqlmock.AnyArg(), sqlmock.AnyArg(), "203.0.113.10", "browser", sqlmock.AnyArg(), sqlmock.AnyArg(), int64(0), "", sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(21, 1)) + adminMock.ExpectExec("UPDATE `external_admin_accounts` SET").WillReturnResult(sqlmock.NewResult(0, 1)) + adminMock.ExpectExec("INSERT INTO `external_admin_login_logs`"). + WithArgs(uint64(7), "fami", "operator", "203.0.113.10", "browser", "success", "login_success", sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(31, 1)) + adminMock.ExpectCommit() + userMock.ExpectQuery("SELECT u.user_id,"). + WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), "fami", int64(9001)). + WillReturnRows(sqlmock.NewRows([]string{ + "user_id", "current_display_user_id", "default_display_user_id", "pretty_display_user_id", "pretty_id", "username", "avatar", "status", + }).AddRow(9001, "123456", "654321", "8888", "8888", "linked-user", "avatar", "active")) + + service := NewService(adminDB, userDB, Config{}) + service.limiter = allowFixedWindowLimiter{} + result, err := service.Login(t.Context(), LoginInput{ + Username: "Operator", Password: "correct-password", IP: "203.0.113.10", UserAgent: "browser", + }) + if err != nil { + t.Fatalf("login without App: %v", err) + } + if result.SessionToken == "" || result.CSRFToken == "" { + t.Fatal("successful login must issue opaque session and CSRF tokens") + } + if result.View.AppCode != "fami" || result.View.App.AppCode != "fami" || result.View.Account.Username != "operator" { + t.Fatalf("resolved session view = %+v", result.View) + } + if err := adminMock.ExpectationsWereMet(); err != nil { + t.Fatalf("admin sql expectations: %v", err) + } + if err := userMock.ExpectationsWereMet(); err != nil { + t.Fatalf("user sql expectations: %v", err) + } } type futureMillis struct{} diff --git a/server/admin/internal/modules/externaladmin/types.go b/server/admin/internal/modules/externaladmin/types.go index f99003e7..88202b49 100644 --- a/server/admin/internal/modules/externaladmin/types.go +++ b/server/admin/internal/modules/externaladmin/types.go @@ -119,7 +119,6 @@ type SessionView struct { } type LoginInput struct { - AppCode string Username string Password string IP string diff --git a/server/admin/migrations/100_external_admin_global_username.sql b/server/admin/migrations/100_external_admin_global_username.sql new file mode 100644 index 00000000..272a28b3 --- /dev/null +++ b/server/admin/migrations/100_external_admin_global_username.sql @@ -0,0 +1,13 @@ +SET NAMES utf8mb4 COLLATE utf8mb4_unicode_ci; + +-- 外管登录不再接受客户端选择 App，因此 username 必须在整张账号表中唯一， +-- 服务端才能仅凭账号确定唯一租户。该 DDL 不改写任何账号，也不会自动重命名冲突凭据： +-- 若历史数据存在同名账号，唯一索引构建会失败并保留原复合索引，要求人工确认归属后再重跑。 +-- external_admin_accounts 是小型凭据表；INPLACE + LOCK=NONE 只扫描该表建立二级索引， +-- 不扫描 users、房间、钱包等业务表，并允许迁移期间继续读写。并发写入若制造同名账号， +-- MySQL 会让索引构建失败而不是提交不唯一结构，保持 fail-closed。 +ALTER TABLE external_admin_accounts + ADD UNIQUE KEY uk_external_admin_accounts_username (username), + DROP INDEX uk_external_admin_accounts_app_username, + ALGORITHM=INPLACE, + LOCK=NONE;