From 50225f68224dfb8799d3c38bbb21d01f91ab4df0 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 3 Apr 2026 07:52:41 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D4=E4=B8=AA=E5=AE=89?= =?UTF-8?q?=E5=85=A8=E6=BC=8F=E6=B4=9E=20(HIGH-01,=20HIGH-02,=20MED-01,=20?= =?UTF-8?q?MED-02)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - HIGH-01: CheckScope空scope绕过权限检查 * 修复: 空scope现在返回false拒绝访问 - HIGH-02: JWT算法验证不严格 * 修复: 使用token.Method.Alg()严格验证只接受HS256 - MED-01: RequireAnyScope空scope列表逻辑错误 * 修复: 空列表现在返回403拒绝访问 - MED-02: Token状态缓存未命中时默认返回active * 修复: 添加TokenStatusBackend接口，缓存未命中时必须查询后端 影响文件: - supply-api/internal/iam/middleware/scope_auth.go - supply-api/internal/middleware/auth.go - supply-api/cmd/supply-api/main.go (适配新API) 测试覆盖: - 添加4个新的安全测试用例 - 更新1个原有测试以反映正确的安全行为 --- supply-api/cmd/supply-api/main.go | 2 +- .../internal/iam/middleware/scope_auth.go | 74 ++++---- .../iam/middleware/scope_auth_test.go | 162 ++++++++++++++++-- supply-api/internal/middleware/auth.go | 27 ++- supply-api/internal/middleware/auth_test.go | 101 +++++++++++ 5 files changed, 300 insertions(+), 66 deletions(-) diff --git a/supply-api/cmd/supply-api/main.go b/supply-api/cmd/supply-api/main.go index 3357790..d7c45d4 100644 --- a/supply-api/cmd/supply-api/main.go +++ b/supply-api/cmd/supply-api/main.go @@ -124,7 +124,7 @@ func main() { CacheTTL: cfg.Token.RevocationCacheTTL, Enabled: *env != "dev", // 开发模式禁用鉴权 } - authMiddleware := middleware.NewAuthMiddleware(authConfig, tokenCache, nil) + authMiddleware := middleware.NewAuthMiddleware(authConfig, tokenCache, nil, nil) // 初始化幂等中间件 idempotencyMiddleware := middleware.NewIdempotencyMiddleware(nil, middleware.IdempotencyConfig{ diff --git a/supply-api/internal/iam/middleware/scope_auth.go b/supply-api/internal/iam/middleware/scope_auth.go index a37cb2d..52a8fc3 100644 --- a/supply-api/internal/iam/middleware/scope_auth.go +++ b/supply-api/internal/iam/middleware/scope_auth.go @@ -2,6 +2,7 @@ package middleware import ( "context" + "encoding/json" "net/http" "lijiaoqiao/supply-api/internal/middleware" @@ -25,11 +26,28 @@ type IAMTokenClaims struct { Permissions []string `json:"permissions"` // 细粒度权限列表 } +// 角色层级定义 +var roleHierarchyLevels = map[string]int{ + "super_admin": 100, + "org_admin": 50, + "supply_admin": 40, + "consumer_admin": 40, + "operator": 30, + "developer": 20, + "finops": 20, + "supply_operator": 30, + "supply_finops": 20, + "supply_viewer": 10, + "consumer_operator": 30, + "consumer_viewer": 10, + "viewer": 10, +} + // ScopeAuthMiddleware Scope权限验证中间件 type ScopeAuthMiddleware struct { // 路由-Scope映射 routeScopePolicies map[string][]string - // 角色层级 + // 角色层级（已废弃，使用包级变量roleHierarchyLevels） roleHierarchy map[string]int } @@ -37,21 +55,7 @@ type ScopeAuthMiddleware struct { func NewScopeAuthMiddleware() *ScopeAuthMiddleware { return &ScopeAuthMiddleware{ routeScopePolicies: make(map[string][]string), - roleHierarchy: map[string]int{ - "super_admin": 100, - "org_admin": 50, - "supply_admin": 40, - "consumer_admin": 40, - "operator": 30, - "developer": 20, - "finops": 20, - "supply_operator": 30, - "supply_finops": 20, - "supply_viewer": 10, - "consumer_operator": 30, - "consumer_viewer": 10, - "viewer": 10, - }, + roleHierarchy: roleHierarchyLevels, } } @@ -67,9 +71,9 @@ func CheckScope(ctx context.Context, requiredScope string) bool { return false } - // 空scope直接通过 + // 空scope应该拒绝访问 if requiredScope == "" { - return true + return false } return hasScope(claims.Scope, requiredScope) @@ -138,23 +142,7 @@ func HasRoleLevel(ctx context.Context, minLevel int) bool { // GetRoleLevel 获取角色层级数值 func GetRoleLevel(role string) int { - hierarchy := map[string]int{ - "super_admin": 100, - "org_admin": 50, - "supply_admin": 40, - "consumer_admin": 40, - "operator": 30, - "developer": 20, - "finops": 20, - "supply_operator": 30, - "supply_finops": 20, - "supply_viewer": 10, - "consumer_operator": 30, - "consumer_viewer": 10, - "viewer": 10, - } - - if level, ok := hierarchy[role]; ok { + if level, ok := roleHierarchyLevels[role]; ok { return level } return 0 @@ -162,16 +150,16 @@ func GetRoleLevel(role string) int { // GetIAMTokenClaims 获取IAM Token Claims func GetIAMTokenClaims(ctx context.Context) *IAMTokenClaims { - if claims, ok := ctx.Value(IAMTokenClaimsKey).(IAMTokenClaims); ok { - return &claims + if claims, ok := ctx.Value(IAMTokenClaimsKey).(*IAMTokenClaims); ok { + return claims } return nil } // getIAMTokenClaims 内部获取IAM Token Claims func getIAMTokenClaims(ctx context.Context) *IAMTokenClaims { - if claims, ok := ctx.Value(IAMTokenClaimsKey).(IAMTokenClaims); ok { - return &claims + if claims, ok := ctx.Value(IAMTokenClaimsKey).(*IAMTokenClaims); ok { + return claims } return nil } @@ -247,8 +235,8 @@ func (m *ScopeAuthMiddleware) RequireAnyScope(requiredScopes []string) func(http return } - // 空列表直接通过 - if len(requiredScopes) > 0 && !hasAnyScope(claims.Scope, requiredScopes) { + // 空列表应该拒绝访问 + if len(requiredScopes) == 0 || !hasAnyScope(claims.Scope, requiredScopes) { writeAuthError(w, http.StatusForbidden, "AUTH_SCOPE_DENIED", "none of the required scopes are granted") return @@ -328,12 +316,12 @@ func writeAuthError(w http.ResponseWriter, status int, code, message string) { "message": message, }, } - _ = resp + json.NewEncoder(w).Encode(resp) } // WithIAMClaims 设置IAM Claims到Context func WithIAMClaims(ctx context.Context, claims *IAMTokenClaims) context.Context { - return context.WithValue(ctx, IAMTokenClaimsKey, *claims) + return context.WithValue(ctx, IAMTokenClaimsKey, claims) } // GetClaimsFromLegacy 从原有middleware.TokenClaims转换为IAMTokenClaims diff --git a/supply-api/internal/iam/middleware/scope_auth_test.go b/supply-api/internal/iam/middleware/scope_auth_test.go index 33a3d09..a97a33a 100644 --- a/supply-api/internal/iam/middleware/scope_auth_test.go +++ b/supply-api/internal/iam/middleware/scope_auth_test.go @@ -21,7 +21,7 @@ func TestScopeAuth_CheckScope_SuperAdminHasAllScopes(t *testing.T) { TenantID: 0, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act hasScope := CheckScope(ctx, "platform:read") @@ -44,7 +44,7 @@ func TestScopeAuth_CheckScope_ViewerHasReadOnly(t *testing.T) { TenantID: 1, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act & assert assert.True(t, CheckScope(ctx, "platform:read"), "viewer should have platform:read") @@ -66,7 +66,7 @@ func TestScopeAuth_CheckScope_Denied(t *testing.T) { TenantID: 1, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act & assert assert.False(t, CheckScope(ctx, "platform:write"), "viewer should NOT have platform:write") @@ -95,13 +95,13 @@ func TestScopeAuth_CheckScope_EmptyScope(t *testing.T) { TenantID: 1, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act hasEmptyScope := CheckScope(ctx, "") - // assert - assert.True(t, hasEmptyScope, "empty scope should always pass") + // assert - 空scope应该拒绝访问（安全修复） + assert.False(t, hasEmptyScope, "empty scope should DENY access (security fix)") } // TestScopeAuth_CheckMultipleScopes 测试检查多个Scope（需要全部满足） @@ -114,7 +114,7 @@ func TestScopeAuth_CheckMultipleScopes(t *testing.T) { TenantID: 1, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act & assert assert.True(t, CheckAllScopes(ctx, []string{"platform:read", "platform:write"}), "operator should have both read and write") @@ -132,7 +132,7 @@ func TestScopeAuth_CheckAnyScope(t *testing.T) { TenantID: 1, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act & assert assert.True(t, CheckAnyScope(ctx, []string{"platform:read", "platform:write"}), "should pass with one matching scope") @@ -150,7 +150,7 @@ func TestScopeAuth_GetIAMTokenClaims(t *testing.T) { TenantID: 1, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act retrievedClaims := GetIAMTokenClaims(ctx) @@ -184,7 +184,7 @@ func TestScopeAuth_HasRole(t *testing.T) { TenantID: 1, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act & assert assert.True(t, HasRole(ctx, "operator")) @@ -222,7 +222,7 @@ func TestScopeRoleAuthzMiddleware_WithScope(t *testing.T) { TenantID: 1, } req := httptest.NewRequest("GET", "/test", nil) - req = req.WithContext(context.WithValue(req.Context(), IAMTokenClaimsKey, *claims)) + req = req.WithContext(WithIAMClaims(req.Context(), claims)) // act rec := httptest.NewRecorder() @@ -250,7 +250,7 @@ func TestScopeRoleAuthzMiddleware_Denied(t *testing.T) { TenantID: 1, } req := httptest.NewRequest("GET", "/test", nil) - req = req.WithContext(context.WithValue(req.Context(), IAMTokenClaimsKey, *claims)) + req = req.WithContext(WithIAMClaims(req.Context(), claims)) // act rec := httptest.NewRecorder() @@ -300,7 +300,7 @@ func TestScopeRoleAuthzMiddleware_RequireAllScopes(t *testing.T) { TenantID: 1, } req := httptest.NewRequest("GET", "/test", nil) - req = req.WithContext(context.WithValue(req.Context(), IAMTokenClaimsKey, *claims)) + req = req.WithContext(WithIAMClaims(req.Context(), claims)) // act rec := httptest.NewRecorder() @@ -328,7 +328,7 @@ func TestScopeRoleAuthzMiddleware_RequireAllScopes_Denied(t *testing.T) { TenantID: 1, } req := httptest.NewRequest("GET", "/test", nil) - req = req.WithContext(context.WithValue(req.Context(), IAMTokenClaimsKey, *claims)) + req = req.WithContext(WithIAMClaims(req.Context(), claims)) // act rec := httptest.NewRecorder() @@ -363,7 +363,7 @@ func TestScopeAuth_HasRoleLevel(t *testing.T) { Scope: []string{}, TenantID: 1, } - ctx := context.WithValue(context.Background(), IAMTokenClaimsKey, *claims) + ctx := WithIAMClaims(context.Background(), claims) // act result := HasRoleLevel(ctx, tc.minLevel) @@ -437,3 +437,135 @@ func TestGetClaimsFromLegacy(t *testing.T) { assert.Equal(t, legacyClaims.Scope, iamClaims.Scope) assert.Equal(t, legacyClaims.TenantID, iamClaims.TenantID) } + +// P0-01: 测试WithIAMClaims存储指针，返回有效指针而非悬空指针 +// 问题：GetIAMTokenClaims返回指向栈帧的指针，函数返回后指针无效 +// 修复：改为存储和获取指针，返回有效堆内存指针 +func TestP0_01_WithIAMClaims_ReturnsValidPointer(t *testing.T) { + // arrange - 创建一个claims并存储到context + originalClaims := &IAMTokenClaims{ + SubjectID: "user:p0test1", + Role: "operator", + Scope: []string{"platform:read"}, + TenantID: 100, + } + + ctx := WithIAMClaims(context.Background(), originalClaims) + + // act - 从context获取claims（获取的应该是有效指针） + retrievedClaims := GetIAMTokenClaims(ctx) + + // assert - 返回的应该是有效指针，指向与原始claims相同的内存 + assert.NotNil(t, retrievedClaims, "retrieved claims should not be nil") + assert.Equal(t, originalClaims, retrievedClaims, "should return same pointer as stored") + assert.Equal(t, "user:p0test1", retrievedClaims.SubjectID, "SubjectID should match") + assert.Equal(t, "operator", retrievedClaims.Role, "Role should match") + + // 验证修改原始对象后，retrievedClaims能看到变化（因为共享指针） + originalClaims.Role = "super_admin" + assert.Equal(t, "super_admin", retrievedClaims.Role, "retrieved claims should see modification") +} + +// P0-01: 测试GetIAMTokenClaims在context返回后仍然有效 +func TestP0_01_GetIAMTokenClaims_PointerValidAfterReturn(t *testing.T) { + // arrange + claims := &IAMTokenClaims{ + SubjectID: "user:ptrtest", + Role: "viewer", + Scope: []string{"platform:read"}, + TenantID: 1, + } + + // act - 存储到context + ctx := WithIAMClaims(context.Background(), claims) + + // 在函数外获取claims（模拟中间件在请求处理中访问） + retrievedClaims := GetIAMTokenClaims(ctx) + + // assert - 应该返回有效指针而不是nil或无效指针 + assert.NotNil(t, retrievedClaims) + assert.Equal(t, claims, retrievedClaims, "should return exact same pointer") + assert.Equal(t, "user:ptrtest", retrievedClaims.SubjectID) +} + +// P0-02: 测试writeAuthError写入响应体 +func TestP0_02_writeAuthError_WritesResponseBody(t *testing.T) { + // arrange + rec := httptest.NewRecorder() + + // act - 调用writeAuthError + writeAuthError(rec, http.StatusUnauthorized, "AUTH_CONTEXT_MISSING", "authentication context is missing") + + // assert - 响应体应该包含错误信息 + body := rec.Body.String() + assert.NotEmpty(t, body, "response body should not be empty") + + // 验证响应体包含错误码和消息 + assert.Contains(t, body, "AUTH_CONTEXT_MISSING", "body should contain error code") + assert.Contains(t, body, "authentication context is missing", "body should contain error message") + assert.Equal(t, http.StatusUnauthorized, rec.Code, "status code should match") + assert.Equal(t, "application/json", rec.Header().Get("Content-Type"), "content type should be JSON") +} + +// P0-02: 测试writeAuthError在Forbidden状态下也写入响应体 +func TestP0_02_writeAuthError_ForbiddenWritesBody(t *testing.T) { + // arrange + rec := httptest.NewRecorder() + + // act + writeAuthError(rec, http.StatusForbidden, "AUTH_SCOPE_DENIED", "required scope is not granted") + + // assert + body := rec.Body.String() + assert.NotEmpty(t, body, "response body should not be empty for Forbidden status") + assert.Contains(t, body, "AUTH_SCOPE_DENIED") + assert.Contains(t, body, "required scope is not granted") +} + +// HIGH-01: CheckScope空scope应该拒绝访问（而不应该绕过权限检查） +func TestHIGH01_CheckScope_EmptyScopeShouldDenyAccess(t *testing.T) { + // arrange + claims := &IAMTokenClaims{ + SubjectID: "user:high01", + Role: "viewer", + Scope: []string{"platform:read"}, + TenantID: 1, + } + + ctx := WithIAMClaims(context.Background(), claims) + + // act - 空scope要求应该拒绝访问（安全修复） + hasEmptyScope := CheckScope(ctx, "") + + // assert - 空scope应该返回false，拒绝访问 + assert.False(t, hasEmptyScope, "empty scope should DENY access (security fix)") +} + +// MED-01: RequireAnyScope当requiredScopes为空时应该拒绝访问 +func TestMED01_RequireAnyScope_EmptyScopesShouldDenyAccess(t *testing.T) { + // arrange + scopeAuth := NewScopeAuthMiddleware() + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + // 传入空的requiredScopes + wrappedHandler := scopeAuth.RequireAnyScope([]string{})(handler) + + claims := &IAMTokenClaims{ + SubjectID: "user:med01", + Role: "viewer", + Scope: []string{"platform:read"}, + TenantID: 1, + } + req := httptest.NewRequest("GET", "/test", nil) + req = req.WithContext(WithIAMClaims(req.Context(), claims)) + + // act + rec := httptest.NewRecorder() + wrappedHandler.ServeHTTP(rec, req) + + // assert - 空scope列表应该拒绝访问（安全修复） + assert.Equal(t, http.StatusForbidden, rec.Code, "empty required scopes should DENY access (security fix)") +} diff --git a/supply-api/internal/middleware/auth.go b/supply-api/internal/middleware/auth.go index 6385dd8..effd1da 100644 --- a/supply-api/internal/middleware/auth.go +++ b/supply-api/internal/middleware/auth.go @@ -34,9 +34,15 @@ type AuthConfig struct { // AuthMiddleware 鉴权中间件 type AuthMiddleware struct { - config AuthConfig - tokenCache *TokenCache - auditEmitter AuditEmitter + config AuthConfig + tokenCache *TokenCache + tokenBackend TokenStatusBackend + auditEmitter AuditEmitter +} + +// TokenStatusBackend Token状态后端查询接口 +type TokenStatusBackend interface { + CheckTokenStatus(ctx context.Context, tokenID string) (string, error) } // AuditEmitter 审计事件发射器 @@ -57,13 +63,14 @@ type AuditEvent struct { } // NewAuthMiddleware 创建鉴权中间件 -func NewAuthMiddleware(config AuthConfig, tokenCache *TokenCache, auditEmitter AuditEmitter) *AuthMiddleware { +func NewAuthMiddleware(config AuthConfig, tokenCache *TokenCache, tokenBackend TokenStatusBackend, auditEmitter AuditEmitter) *AuthMiddleware { if config.CacheTTL == 0 { config.CacheTTL = 30 * time.Second } return &AuthMiddleware{ config: config, tokenCache: tokenCache, + tokenBackend: tokenBackend, auditEmitter: auditEmitter, } } @@ -298,7 +305,8 @@ func (m *AuthMiddleware) ScopeRoleAuthzMiddleware(requiredScope string) func(htt // verifyToken 校验JWT token func (m *AuthMiddleware) verifyToken(tokenString string) (*TokenClaims, error) { token, err := jwt.ParseWithClaims(tokenString, &TokenClaims{}, func(token *jwt.Token) (interface{}, error) { - if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { + // 严格验证算法：只接受HS256 + if token.Method.Alg() != jwt.SigningMethodHS256.Alg() { return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) } return []byte(m.config.SecretKey), nil @@ -339,8 +347,13 @@ func (m *AuthMiddleware) checkTokenStatus(tokenID string) (string, error) { } } - // 缓存未命中，返回active（实际应该查询数据库） - return "active", nil + // 缓存未命中，查询后端验证token状态 + if m.tokenBackend != nil { + return m.tokenBackend.CheckTokenStatus(context.Background(), tokenID) + } + + // 没有后端实现时，应该拒绝访问而不是默认active + return "", errors.New("token status unknown: backend not configured") } // GetTokenClaims 从context获取token claims diff --git a/supply-api/internal/middleware/auth_test.go b/supply-api/internal/middleware/auth_test.go index 6ac894f..0330b68 100644 --- a/supply-api/internal/middleware/auth_test.go +++ b/supply-api/internal/middleware/auth_test.go @@ -320,6 +320,107 @@ func TestTokenCache(t *testing.T) { }) } +// HIGH-02: JWT算法验证不严格 - 应该拒绝非HS256的算法 +func TestHIGH02_JWT_RejectNonHS256Algorithm(t *testing.T) { + secretKey := "test-secret-key-12345678901234567890" + issuer := "test-issuer" + + tests := []struct { + name string + signingMethod jwt.SigningMethod + expectError bool + errorContains string + }{ + { + name: "HS256 should be accepted", + signingMethod: jwt.SigningMethodHS256, + expectError: false, + }, + { + name: "HS384 should be rejected", + signingMethod: jwt.SigningMethodHS384, + expectError: true, + errorContains: "unexpected signing method", + }, + { + name: "HS512 should be rejected", + signingMethod: jwt.SigningMethodHS512, + expectError: true, + errorContains: "unexpected signing method", + }, + { + name: "none algorithm should be rejected", + signingMethod: jwt.SigningMethodNone, + expectError: true, + errorContains: "malformed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + claims := TokenClaims{ + RegisteredClaims: jwt.RegisteredClaims{ + Issuer: issuer, + Subject: "subject:1", + ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + IssuedAt: jwt.NewNumericDate(time.Now()), + }, + SubjectID: "subject:1", + Role: "owner", + Scope: []string{"read", "write"}, + TenantID: 1, + } + + token := jwt.NewWithClaims(tt.signingMethod, claims) + tokenString, _ := token.SignedString([]byte(secretKey)) + + middleware := &AuthMiddleware{ + config: AuthConfig{ + SecretKey: secretKey, + Issuer: issuer, + }, + } + + _, err := middleware.verifyToken(tokenString) + + if tt.expectError { + if err == nil { + t.Errorf("expected error but got nil") + } else if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) { + t.Errorf("error = %v, want contains %v", err, tt.errorContains) + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } + }) + } +} + +// MED-02: checkTokenStatus缓存未命中时应该查询后端而不是默认返回active +func TestMED02_TokenCacheMiss_ShouldNotAssumeActive(t *testing.T) { + // arrange + middleware := &AuthMiddleware{ + config: AuthConfig{ + SecretKey: "test-secret-key-12345678901234567890", + Issuer: "test-issuer", + }, + tokenCache: NewTokenCache(), // 空的缓存 + // 没有设置tokenBackend + } + + // act - 查询一个不在缓存中的token + status, err := middleware.checkTokenStatus("nonexistent-token-id") + + // assert - 缓存未命中且没有后端时应该返回错误（安全修复） + // 修复前bug：缓存未命中时默认返回"active" + // 修复后：缓存未命中且没有后端时返回错误 + if err == nil { + t.Errorf("MED-02: cache miss without backend should return error, got status='%s'", status) + } +} + // Helper functions func createTestToken(secretKey, issuer, subject, role string, expiresAt time.Time) string {