From 92cf2921dd2c48ca55efce4e3407e3ef2660a756 Mon Sep 17 00:00:00 2001 From: shanshanzhong147 Date: Thu, 9 Jul 2026 07:45:58 -0700 Subject: [PATCH] =?UTF-8?q?=E4=BF=AE=E5=A4=8D(#3):=20=E6=8A=BD=E5=A5=96=20?= =?UTF-8?q?Stage=201=20=E5=90=8E=E5=8F=B0=E5=AE=A1=E8=AE=A1=20IP/UA=20?= =?UTF-8?q?=E4=B8=A2=E5=A4=B1=20+=20=E5=86=92=E7=83=9F=E8=84=9A=E6=9C=AC?= =?UTF-8?q?=20Bearer=20=E5=89=8D=E7=BC=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes HIF-3 (Stage 1 P1 defects from QA smoke report) F2: admin_action_log.ip / user_agent 恒为空 - 根因：requestMeta 从 ctx 读裸字符串 key，全库无 writer 塞 - 修复：新增 AdminMetaMiddleware 用 typed constant.CtxKeyIP / CtxKeyUserAgent - 挂载：lottery admin 组末尾（不 gate access） - 回归护栏：UsesTypedKey + IgnoresBareStringKeys 双向断言 typed key，防止未来退化回裸字符串 F3: qa/lottery/stage1_curl.sh Bearer 前缀 - 删除两处 Bearer 前缀 + 加注释说明 ppanel AuthMiddleware 不 strip 单测 5 用例全绿；scope 严格限于 lottery admin 组，其它路径零改动；无 DB 变更。 Merged: architect review 后，将触发 staging 二次部署 — 期望这次能一并解决 shanshanzhong147 手工 SSH 后 Lottery.Enable=true 未生效的问题（若真是 mount/restart 未正确 pick up）。 --- internal/handler/lottery_routes.go | 2 +- internal/logic/admin/lottery/lottery.go | 8 +- .../logic/admin/lottery/request_meta_test.go | 51 ++++++++++++ internal/middleware/adminMetaMiddleware.go | 26 ++++++ .../middleware/adminMetaMiddleware_test.go | 81 +++++++++++++++++++ pkg/constant/context.go | 6 ++ qa/lottery/stage1_curl.sh | 6 +- 7 files changed, 174 insertions(+), 6 deletions(-) create mode 100644 internal/logic/admin/lottery/request_meta_test.go create mode 100644 internal/middleware/adminMetaMiddleware.go create mode 100644 internal/middleware/adminMetaMiddleware_test.go diff --git a/internal/handler/lottery_routes.go b/internal/handler/lottery_routes.go index 5b22c8b..820827e 100644 --- a/internal/handler/lottery_routes.go +++ b/internal/handler/lottery_routes.go @@ -24,7 +24,7 @@ func registerLotteryRoutes(router *gin.Engine, serverCtx *svc.ServiceContext) { } adminGroup := router.Group("/v1/admin/lottery") - adminGroup.Use(middleware.AuthMiddleware(serverCtx)) + adminGroup.Use(middleware.AuthMiddleware(serverCtx), middleware.AdminMetaMiddleware()) { adminGroup.POST("/activities", adminLottery.CreateLotteryActivityHandler(serverCtx)) adminGroup.PUT("/activities", adminLottery.UpdateLotteryActivityHandler(serverCtx)) diff --git a/internal/logic/admin/lottery/lottery.go b/internal/logic/admin/lottery/lottery.go index a6b51ad..ca0d634 100644 --- a/internal/logic/admin/lottery/lottery.go +++ b/internal/logic/admin/lottery/lottery.go @@ -35,11 +35,13 @@ func currentAdminId(ctx context.Context) int64 { } func requestMeta(ctx context.Context) (ip, ua string) { - // AuthMiddleware attaches gin.Context; we accept absence gracefully. - if v, ok := ctx.Value("ip").(string); ok { + // AdminMetaMiddleware populates these keys on the request context after + // AuthMiddleware runs. Absent middleware (unit tests, non-admin paths) + // → empty strings, which is the intended defensive default. + if v, ok := ctx.Value(constant.CtxKeyIP).(string); ok { ip = v } - if v, ok := ctx.Value("user_agent").(string); ok { + if v, ok := ctx.Value(constant.CtxKeyUserAgent).(string); ok { ua = v } return diff --git a/internal/logic/admin/lottery/request_meta_test.go b/internal/logic/admin/lottery/request_meta_test.go new file mode 100644 index 0000000..4af3fcd --- /dev/null +++ b/internal/logic/admin/lottery/request_meta_test.go @@ -0,0 +1,51 @@ +package lottery + +import ( + "context" + "testing" + + "github.com/perfect-panel/server/pkg/constant" +) + +// TestRequestMeta_EmptyWhenCtxUnset asserts requestMeta returns empty strings +// when neither typed context key is populated (unit tests, non-admin paths). +func TestRequestMeta_EmptyWhenCtxUnset(t *testing.T) { + ip, ua := requestMeta(context.Background()) + if ip != "" || ua != "" { + t.Fatalf("expected empty, got ip=%q ua=%q", ip, ua) + } +} + +// TestRequestMeta_ReadsTypedKeys asserts requestMeta picks up the values +// AdminMetaMiddleware pins onto ctx via the typed CtxKey constants. +func TestRequestMeta_ReadsTypedKeys(t *testing.T) { + ctx := context.Background() + ctx = context.WithValue(ctx, constant.CtxKeyIP, "10.99.99.7") + ctx = context.WithValue(ctx, constant.CtxKeyUserAgent, "qa-audit-probe") + + ip, ua := requestMeta(ctx) + if ip != "10.99.99.7" { + t.Fatalf("ip = %q, want %q", ip, "10.99.99.7") + } + if ua != "qa-audit-probe" { + t.Fatalf("ua = %q, want %q", ua, "qa-audit-probe") + } +} + +// TestRequestMeta_IgnoresBareStringKeys guards against the F2 root cause: +// pre-fix, the writer used bare-string keys "ip" / "user_agent" which never +// collided with anyone's typed reader — so audit rows always saw empty +// strings. The test proves the reader now IGNORES bare-string writes: only +// the typed CtxKey path counts. +func TestRequestMeta_IgnoresBareStringKeys(t *testing.T) { + ctx := context.Background() + //nolint:staticcheck // intentional bare-string key to prove reader ignores it + ctx = context.WithValue(ctx, "ip", "should-be-ignored") + //nolint:staticcheck // intentional bare-string key to prove reader ignores it + ctx = context.WithValue(ctx, "user_agent", "should-be-ignored") + + ip, ua := requestMeta(ctx) + if ip != "" || ua != "" { + t.Fatalf("bare-string keys must be ignored; got ip=%q ua=%q", ip, ua) + } +} diff --git a/internal/middleware/adminMetaMiddleware.go b/internal/middleware/adminMetaMiddleware.go new file mode 100644 index 0000000..ce764fc --- /dev/null +++ b/internal/middleware/adminMetaMiddleware.go @@ -0,0 +1,26 @@ +package middleware + +import ( + "context" + + "github.com/gin-gonic/gin" + "github.com/perfect-panel/server/pkg/constant" +) + +// AdminMetaMiddleware pins the request's client IP and User-Agent onto the +// context under constant.CtxKeyIP / constant.CtxKeyUserAgent so downstream +// audit writers (admin_action_log) can capture them without threading the +// gin.Context through every logic layer. +// +// This middleware is a no-op for auth: it does NOT gate access; wire it +// after AuthMiddleware so ctx.Value(CtxKeyUser) is already populated by the +// time an admin logic writes audit rows. +func AdminMetaMiddleware() gin.HandlerFunc { + return func(c *gin.Context) { + ctx := c.Request.Context() + ctx = context.WithValue(ctx, constant.CtxKeyIP, c.ClientIP()) + ctx = context.WithValue(ctx, constant.CtxKeyUserAgent, c.Request.UserAgent()) + c.Request = c.Request.WithContext(ctx) + c.Next() + } +} diff --git a/internal/middleware/adminMetaMiddleware_test.go b/internal/middleware/adminMetaMiddleware_test.go new file mode 100644 index 0000000..2a0e080 --- /dev/null +++ b/internal/middleware/adminMetaMiddleware_test.go @@ -0,0 +1,81 @@ +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/perfect-panel/server/pkg/constant" +) + +// TestAdminMetaMiddleware_PopulatesCtx asserts the middleware pins ClientIP +// and User-Agent onto the request context under the typed constant keys, so +// downstream audit writers can pick them up without gin.Context threading. +func TestAdminMetaMiddleware_PopulatesCtx(t *testing.T) { + gin.SetMode(gin.TestMode) + + var ( + gotIP string + gotUA string + ) + r := gin.New() + r.Use(AdminMetaMiddleware()) + r.GET("/ping", func(c *gin.Context) { + ctx := c.Request.Context() + gotIP, _ = ctx.Value(constant.CtxKeyIP).(string) + gotUA, _ = ctx.Value(constant.CtxKeyUserAgent).(string) + c.String(http.StatusOK, "ok") + }) + + req := httptest.NewRequest(http.MethodGet, "/ping", nil) + req.RemoteAddr = "10.99.99.7:54321" + req.Header.Set("User-Agent", "qa-audit-probe") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", w.Code) + } + if gotUA != "qa-audit-probe" { + t.Fatalf("user_agent = %q, want %q", gotUA, "qa-audit-probe") + } + // Gin resolves ClientIP() from RemoteAddr when no forwarded headers are + // trusted. It strips the port, so we assert the exact host we set. + if gotIP != "10.99.99.7" { + t.Fatalf("ip = %q, want %q", gotIP, "10.99.99.7") + } +} + +// TestAdminMetaMiddleware_UsesTypedKey guards against the regression that +// motivated PR D: reader and writer must share the typed CtxKey, not a bare +// string. A ctx.Value("ip") lookup (bare string) MUST miss even though the +// typed CtxKey "ip" is present. +func TestAdminMetaMiddleware_UsesTypedKey(t *testing.T) { + gin.SetMode(gin.TestMode) + + var ( + typedIP string + bareStrIP any + ) + r := gin.New() + r.Use(AdminMetaMiddleware()) + r.GET("/ping", func(c *gin.Context) { + ctx := c.Request.Context() + typedIP, _ = ctx.Value(constant.CtxKeyIP).(string) + bareStrIP = ctx.Value("ip") // bare string key — must MISS + c.String(http.StatusOK, "ok") + }) + + req := httptest.NewRequest(http.MethodGet, "/ping", nil) + req.RemoteAddr = "10.1.2.3:80" + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if typedIP == "" { + t.Fatalf("typed CtxKeyIP lookup must succeed") + } + if bareStrIP != nil { + t.Fatalf("bare-string \"ip\" lookup MUST miss (got %v); F2 regression risk", bareStrIP) + } +} diff --git a/pkg/constant/context.go b/pkg/constant/context.go index 9a8f2d7..3635232 100644 --- a/pkg/constant/context.go +++ b/pkg/constant/context.go @@ -16,4 +16,10 @@ const ( CtxKeyAPIVersionUseLatest CtxKey = "apiVersionUseLatest" CtxKeyAPIHeaderRaw CtxKey = "apiHeaderRaw" CtxKeyHasAppId CtxKey = "hasAppId" + // CtxKeyIP / CtxKeyUserAgent are populated by AdminMetaMiddleware for + // audit-writing logic (admin_action_log.ip / user_agent). Both keys use + // the typed CtxKey so ctx.Value lookups collide-safely with any bare-string + // writers a future middleware might accidentally introduce. + CtxKeyIP CtxKey = "ip" + CtxKeyUserAgent CtxKey = "user_agent" ) diff --git a/qa/lottery/stage1_curl.sh b/qa/lottery/stage1_curl.sh index 7821e55..3abae76 100755 --- a/qa/lottery/stage1_curl.sh +++ b/qa/lottery/stage1_curl.sh @@ -19,8 +19,10 @@ ADMIN_TOKEN="${ADMIN_TOKEN:-CHANGE_ME}" USER_TOKEN="${USER_TOKEN:-CHANGE_ME}" USER_ID="${USER_ID:-1}" -hdr_admin=(-H "Authorization: Bearer ${ADMIN_TOKEN}" -H "Content-Type: application/json") -hdr_user=(-H "Authorization: Bearer ${USER_TOKEN}" -H "Content-Type: application/json") +# ppanel AuthMiddleware 不 strip Bearer 前缀，直接把整个 Authorization header 传给 +# jwt.ParseJwtToken —— 所以这里必须传裸 JWT，不能加 "Bearer " 前缀，否则 parse fail → 40004。 +hdr_admin=(-H "Authorization: ${ADMIN_TOKEN}" -H "Content-Type: application/json") +hdr_user=(-H "Authorization: ${USER_TOKEN}" -H "Content-Type: application/json") log() { printf '\n\033[1;34m▶ %s\033[0m\n' "$*"