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' "$*"