From 56c62c59c813d8232c037b2f49c910982b672a24 Mon Sep 17 00:00:00 2001 From: "Bestony@Homelab" Date: Tue, 16 Jun 2026 16:58:27 +0800 Subject: [PATCH] fix(auth): include client ip in acl denial message --- .../server/middleware/api_key_auth.go | 6 +- .../server/middleware/api_key_auth_test.go | 112 +++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/backend/internal/server/middleware/api_key_auth.go b/backend/internal/server/middleware/api_key_auth.go index 32257b0bae..2f0a3f1cf7 100644 --- a/backend/internal/server/middleware/api_key_auth.go +++ b/backend/internal/server/middleware/api_key_auth.go @@ -3,6 +3,7 @@ package middleware import ( "context" "errors" + "fmt" "strings" "github.com/Wei-Shaw/sub2api/internal/config" @@ -99,8 +100,11 @@ func apiKeyAuthWithSubscription(apiKeyService *service.APIKeyService, subscripti } allowed, _ := ip.CheckIPRestrictionWithCompiledRules(clientIP, apiKey.CompiledIPWhitelist, apiKey.CompiledIPBlacklist) if !allowed { + if clientIP == "" { + clientIP = "unknown" + } service.MarkOpsClientBusinessLimited(c, service.OpsClientBusinessLimitedReasonIPRestriction) - AbortWithError(c, 403, "ACCESS_DENIED", "Access denied") + AbortWithError(c, 403, "ACCESS_DENIED", fmt.Sprintf("Access denied. Your IP is %s", clientIP)) return } } diff --git a/backend/internal/server/middleware/api_key_auth_test.go b/backend/internal/server/middleware/api_key_auth_test.go index dc445b3cef..d2234e8370 100644 --- a/backend/internal/server/middleware/api_key_auth_test.go +++ b/backend/internal/server/middleware/api_key_auth_test.go @@ -4,6 +4,7 @@ package middleware import ( "context" + "encoding/json" "errors" "net/http" "net/http/httptest" @@ -702,11 +703,59 @@ func TestAPIKeyAuthIPRestrictionDoesNotTrustForwardedClientIPByDefault(t *testin router.ServeHTTP(w, req) require.Equal(t, http.StatusForbidden, w.Code) - require.Contains(t, w.Body.String(), "ACCESS_DENIED") + requireAPIKeyAuthError(t, w, "ACCESS_DENIED", "Access denied. Your IP is 9.9.9.9") require.True(t, markedBusinessLimited) require.Equal(t, service.OpsClientBusinessLimitedReasonIPRestriction, businessLimitedReason) } +func TestAPIKeyAuthIPRestrictionIncludesClientIPForBlacklistDenial(t *testing.T) { + gin.SetMode(gin.TestMode) + + user := &service.User{ + ID: 7, + Role: service.RoleUser, + Status: service.StatusActive, + Balance: 10, + Concurrency: 3, + } + apiKey := &service.APIKey{ + ID: 100, + UserID: user.ID, + Key: "test-key", + Status: service.StatusActive, + User: user, + IPBlacklist: []string{"9.9.9.9"}, + } + + apiKeyRepo := &stubApiKeyRepo{ + getByKey: func(ctx context.Context, key string) (*service.APIKey, error) { + if key != apiKey.Key { + return nil, service.ErrAPIKeyNotFound + } + clone := *apiKey + return &clone, nil + }, + } + + cfg := &config.Config{RunMode: config.RunModeSimple} + apiKeyService := service.NewAPIKeyService(apiKeyRepo, nil, nil, nil, nil, nil, cfg) + router := gin.New() + require.NoError(t, router.SetTrustedProxies(nil)) + router.Use(gin.HandlerFunc(NewAPIKeyAuthMiddleware(apiKeyService, nil, cfg))) + router.GET("/t", func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"ok": true}) + }) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/t", nil) + req.RemoteAddr = "9.9.9.9:12345" + req.Header.Set("x-api-key", apiKey.Key) + router.ServeHTTP(w, req) + + require.Equal(t, http.StatusForbidden, w.Code) + requireAPIKeyAuthError(t, w, "ACCESS_DENIED", "Access denied. Your IP is 9.9.9.9") +} + func TestAPIKeyAuthIPRestrictionCanTrustForwardedClientIPForReverseProxy(t *testing.T) { gin.SetMode(gin.TestMode) @@ -758,6 +807,58 @@ func TestAPIKeyAuthIPRestrictionCanTrustForwardedClientIPForReverseProxy(t *test require.Equal(t, http.StatusOK, w.Code) } +func TestAPIKeyAuthIPRestrictionUsesForwardedClientIPInDenialWhenTrusted(t *testing.T) { + gin.SetMode(gin.TestMode) + + user := &service.User{ + ID: 7, + Role: service.RoleUser, + Status: service.StatusActive, + Balance: 10, + Concurrency: 3, + } + apiKey := &service.APIKey{ + ID: 100, + UserID: user.ID, + Key: "test-key", + Status: service.StatusActive, + User: user, + IPWhitelist: []string{"9.9.9.9"}, + } + + apiKeyRepo := &stubApiKeyRepo{ + getByKey: func(ctx context.Context, key string) (*service.APIKey, error) { + if key != apiKey.Key { + return nil, service.ErrAPIKeyNotFound + } + clone := *apiKey + return &clone, nil + }, + } + + cfg := &config.Config{RunMode: config.RunModeSimple} + cfg.SetTrustForwardedIPForAPIKeyACL(true) + apiKeyService := service.NewAPIKeyService(apiKeyRepo, nil, nil, nil, nil, nil, cfg) + router := gin.New() + require.NoError(t, router.SetTrustedProxies(nil)) + router.Use(gin.HandlerFunc(NewAPIKeyAuthMiddleware(apiKeyService, nil, cfg))) + router.GET("/t", func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"ok": true}) + }) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/t", nil) + req.RemoteAddr = "9.9.9.9:12345" + req.Header.Set("x-api-key", apiKey.Key) + req.Header.Set("X-Forwarded-For", "1.2.3.4") + req.Header.Set("X-Real-IP", "1.2.3.4") + req.Header.Set("CF-Connecting-IP", "1.2.3.4") + router.ServeHTTP(w, req) + + require.Equal(t, http.StatusForbidden, w.Code) + requireAPIKeyAuthError(t, w, "ACCESS_DENIED", "Access denied. Your IP is 1.2.3.4") +} + func TestAPIKeyAuthTouchesLastUsedOnSuccess(t *testing.T) { gin.SetMode(gin.TestMode) @@ -908,6 +1009,15 @@ func newAuthTestRouter(apiKeyService *service.APIKeyService, subscriptionService return router } +func requireAPIKeyAuthError(t *testing.T, w *httptest.ResponseRecorder, code, message string) { + t.Helper() + + var resp ErrorResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + require.Equal(t, code, resp.Code) + require.Equal(t, message, resp.Message) +} + type stubApiKeyRepo struct { getByKey func(ctx context.Context, key string) (*service.APIKey, error) updateLastUsed func(ctx context.Context, id int64, usedAt time.Time) error