Merge pull request #1220 from trheyi/main

Enhance ACL configuration and role management integration

openapi/oauth/acl/acl.go

@@ -2,6 +2,7 @@ package acl
 
 import (
 	"github.com/yaoapp/kun/log"
+	"github.com/yaoapp/yao/openapi/oauth/acl/role"
 )
 
 // Global is the global ACL enforcer
@@ -28,6 +29,10 @@ func New(config *Config) (Enforcer, error) {
 		}
 		acl.Scope = manager
 		log.Info("[ACL] Scope manager loaded successfully")
+
+		// Init Role Manager
+		role.RoleManager = role.NewManager(config.Cache, config.Provider)
+		log.Info("[ACL] Role manager loaded successfully")
 	}
 
 	return acl, nil
@@ -39,6 +44,16 @@ func Load(config *Config) (Enforcer, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	// Clear role cache after loading to ensure fresh data
+	if config != nil && config.Enabled && role.RoleManager != nil {
+		if err := role.RoleManager.ClearCache(); err != nil {
+			log.Warn("[ACL] Failed to clear role cache after loading: %v", err)
+		} else {
+			log.Debug("[ACL] Role cache cleared successfully")
+		}
+	}
+
 	Global = enforcer
 	return Global, nil
 }

openapi/oauth/acl/role/cache.go

@@ -0,0 +1,244 @@
+package role
+
+import (
+	"fmt"
+	"time"
+)
+
+// PRE prefix for the role cache
+const PRE = "acl:role:"
+
+// TTL time for the role cache
+const TTL = 1 * time.Hour
+
+// keyUserRole returns the key for the user role cache
+func (m *Manager) keyUserRole(userID string) string {
+	return fmt.Sprintf("%suser:%s", PRE, userID)
+}
+
+// keyClientRole returns the key for the client role cache
+func (m *Manager) keyClientRole(clientID string) string {
+	return fmt.Sprintf("%sclient:%s", PRE, clientID)
+}
+
+// keyTeamRole returns the key for the team role cache
+func (m *Manager) keyTeamRole(teamID string) string {
+	return fmt.Sprintf("%steam:%s", PRE, teamID)
+}
+
+// keyMemberRole returns the key for the member role cache
+func (m *Manager) keyMemberRole(teamID, userID string) string {
+	return fmt.Sprintf("%smember:%s:%s", PRE, teamID, userID)
+}
+
+// keyScopes returns the key for the allowed scopes cache
+func (m *Manager) keyScopes(roleID string) string {
+	return fmt.Sprintf("%sscopes:%s", PRE, roleID)
+}
+
+// keyScopesRestricted returns the key for the restricted scopes cache
+func (m *Manager) keyScopesRestricted(roleID string) string {
+	return fmt.Sprintf("%sscopes:restricted:%s", PRE, roleID)
+}
+
+// ============================================================================
+// Cache Get Operations
+// ============================================================================
+
+// getUserRoleCache gets the user role from the cache
+func (m *Manager) getUserRoleCache(userID string) (string, bool) {
+	if m.cache == nil {
+		return "", false
+	}
+	value, has := m.cache.Get(m.keyUserRole(userID))
+	if !has {
+		return "", false
+	}
+	return toString(value), true
+}
+
+// getClientRoleCache gets the client role from the cache
+func (m *Manager) getClientRoleCache(clientID string) (string, bool) {
+	if m.cache == nil {
+		return "", false
+	}
+	value, has := m.cache.Get(m.keyClientRole(clientID))
+	if !has {
+		return "", false
+	}
+	return toString(value), true
+}
+
+// getTeamRoleCache gets the team role from the cache
+func (m *Manager) getTeamRoleCache(teamID string) (string, bool) {
+	if m.cache == nil {
+		return "", false
+	}
+	value, has := m.cache.Get(m.keyTeamRole(teamID))
+	if !has {
+		return "", false
+	}
+	return toString(value), true
+}
+
+// getMemberRoleCache gets the member role from the cache
+func (m *Manager) getMemberRoleCache(teamID, userID string) (string, bool) {
+	if m.cache == nil {
+		return "", false
+	}
+	value, has := m.cache.Get(m.keyMemberRole(teamID, userID))
+	if !has {
+		return "", false
+	}
+	return toString(value), true
+}
+
+// getScopesCache gets the scopes from the cache
+// Returns: (allowedScopes, restrictedScopes, found)
+func (m *Manager) getScopesCache(roleID string) ([]string, []string, bool) {
+	if m.cache == nil {
+		return nil, nil, false
+	}
+
+	// Get allowed scopes
+	allowedValue, hasAllowed := m.cache.Get(m.keyScopes(roleID))
+	if !hasAllowed {
+		return nil, nil, false
+	}
+
+	// Get restricted scopes
+	restrictedValue, hasRestricted := m.cache.Get(m.keyScopesRestricted(roleID))
+	// Note: restricted scopes might not exist, which is OK
+
+	allowedScopes := toStringArray(allowedValue)
+	restrictedScopes := []string{}
+	if hasRestricted {
+		restrictedScopes = toStringArray(restrictedValue)
+	}
+
+	return allowedScopes, restrictedScopes, true
+}
+
+// ============================================================================
+// Cache Set Operations
+// ============================================================================
+
+// setUserRoleCache sets the user role in the cache
+func (m *Manager) setUserRoleCache(userID, roleID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Set(m.keyUserRole(userID), roleID, TTL)
+}
+
+// setClientRoleCache sets the client role in the cache
+func (m *Manager) setClientRoleCache(clientID, roleID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Set(m.keyClientRole(clientID), roleID, TTL)
+}
+
+// setTeamRoleCache sets the team role in the cache
+func (m *Manager) setTeamRoleCache(teamID, roleID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Set(m.keyTeamRole(teamID), roleID, TTL)
+}
+
+// setMemberRoleCache sets the member role in the cache
+func (m *Manager) setMemberRoleCache(teamID, userID, roleID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Set(m.keyMemberRole(teamID, userID), roleID, TTL)
+}
+
+// setScopesCache sets the scopes in the cache
+func (m *Manager) setScopesCache(roleID string, allowedScopes []string, restrictedScopes []string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+
+	// Set allowed scopes
+	err := m.cache.Set(m.keyScopes(roleID), allowedScopes, TTL)
+	if err != nil {
+		return err
+	}
+
+	// Set restricted scopes
+	err = m.cache.Set(m.keyScopesRestricted(roleID), restrictedScopes, TTL)
+	if err != nil {
+		// If setting restricted scopes fails, delete the allowed scopes to maintain consistency
+		_ = m.cache.Del(m.keyScopes(roleID))
+		return err
+	}
+
+	return nil
+}
+
+// ============================================================================
+// Cache Delete Operations
+// ============================================================================
+
+// delUserRoleCache deletes the user role from the cache
+func (m *Manager) delUserRoleCache(userID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Del(m.keyUserRole(userID))
+}
+
+// delClientRoleCache deletes the client role from the cache
+func (m *Manager) delClientRoleCache(clientID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Del(m.keyClientRole(clientID))
+}
+
+// delTeamRoleCache deletes the team role from the cache
+func (m *Manager) delTeamRoleCache(teamID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Del(m.keyTeamRole(teamID))
+}
+
+// delMemberRoleCache deletes the member role from the cache
+func (m *Manager) delMemberRoleCache(teamID, userID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Del(m.keyMemberRole(teamID, userID))
+}
+
+// delScopesCache deletes the scopes from the cache
+func (m *Manager) delScopesCache(roleID string) error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+
+	// Delete allowed scopes
+	err := m.cache.Del(m.keyScopes(roleID))
+	if err != nil {
+		return err
+	}
+
+	// Delete restricted scopes
+	err = m.cache.Del(m.keyScopesRestricted(roleID))
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// ClearCache clears the role cache
+func (m *Manager) ClearCache() error {
+	if m.cache == nil {
+		return nil // Silently skip if cache is not configured
+	}
+	return m.cache.Del(fmt.Sprintf("%s*", PRE))
+}

openapi/oauth/acl/role/role.go

@@ -0,0 +1,253 @@
+package role
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/yaoapp/gou/store"
+	"github.com/yaoapp/yao/openapi/oauth/types"
+)
+
+// RoleManager is the global role manager
+var RoleManager *Manager = nil
+
+// NewManager creates a new role manager
+func NewManager(cache store.Store, provider types.UserProvider) *Manager {
+	return &Manager{
+		cache:    cache,
+		provider: provider,
+	}
+}
+
+// GetClientRole gets the role for a client
+func (m *Manager) GetClientRole(ctx context.Context, clientID string) (string, error) {
+
+	// Get From Cache
+	roleID, has := m.getClientRoleCache(clientID)
+	if has {
+		return roleID, nil
+	}
+
+	// Get From Database
+	role, err := m.getClientRole(ctx, clientID)
+	if err != nil {
+		return "", err
+	}
+
+	// Set Cache
+	err = m.setClientRoleCache(clientID, role)
+	if err != nil {
+		return "", err
+	}
+
+	return role, nil
+}
+
+// GetUserRole gets the role for a user
+func (m *Manager) GetUserRole(ctx context.Context, userID string) (string, error) {
+	// Get From Cache
+	roleID, has := m.getUserRoleCache(userID)
+	if has {
+		return roleID, nil
+	}
+
+	// Get From Database using UserProvider
+	role, err := m.getUserRole(ctx, userID)
+	if err != nil {
+		return "", err
+	}
+
+	// Set Cache
+	err = m.setUserRoleCache(userID, role)
+	if err != nil {
+		return "", err
+	}
+
+	return role, nil
+}
+
+// GetTeamRole gets the role for a team
+func (m *Manager) GetTeamRole(ctx context.Context, teamID string) (string, error) {
+	// Get From Cache
+	roleID, has := m.getTeamRoleCache(teamID)
+	if has {
+		return roleID, nil
+	}
+
+	// Get From Database using UserProvider
+	role, err := m.getTeamRole(ctx, teamID)
+	if err != nil {
+		return "", err
+	}
+
+	// Set Cache
+	err = m.setTeamRoleCache(teamID, role)
+	if err != nil {
+		return "", err
+	}
+
+	return role, nil
+}
+
+// GetMemberRole gets the role for a member
+func (m *Manager) GetMemberRole(ctx context.Context, teamID, userID string) (string, error) {
+	// Get From Cache
+	roleID, has := m.getMemberRoleCache(teamID, userID)
+	if has {
+		return roleID, nil
+	}
+
+	// Get From Database using UserProvider
+	role, err := m.getMemberRole(ctx, teamID, userID)
+	if err != nil {
+		return "", err
+	}
+
+	// Set Cache
+	err = m.setMemberRoleCache(teamID, userID, role)
+	if err != nil {
+		return "", err
+	}
+
+	return role, nil
+}
+
+// ============================================================================
+// Scope Resource
+// ============================================================================
+
+// GetScopes gets the scopes for a role
+// Returns: (allowedScopes, restrictedScopes, error)
+func (m *Manager) GetScopes(ctx context.Context, roleID string) ([]string, []string, error) {
+	// Get From Cache
+	allowed, restricted, has := m.getScopesCache(roleID)
+	if has {
+		return allowed, restricted, nil
+	}
+
+	// Get From Database using UserProvider
+	allowedScopes, restrictedScopes, err := m.getScopes(ctx, roleID)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// Set Cache
+	err = m.setScopesCache(roleID, allowedScopes, restrictedScopes)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return allowedScopes, restrictedScopes, nil
+}
+
+// ============================================================================
+// Role Resource - Private Methods
+// ============================================================================
+
+// getClientRole gets the role for a client from database
+func (m *Manager) getClientRole(ctx context.Context, clientID string) (string, error) {
+	// TODO: Implement client role retrieval from ClientProvider
+	// For now, return a default role
+	return "system:root", nil
+}
+
+// getUserRole gets the role for a user from database
+func (m *Manager) getUserRole(ctx context.Context, userID string) (string, error) {
+	if m.provider == nil {
+		return "", fmt.Errorf("user provider is not configured")
+	}
+
+	// Get user role information
+	roleInfo, err := m.provider.GetUserRole(ctx, userID)
+	if err != nil {
+		return "", fmt.Errorf("failed to get user role: %w", err)
+	}
+
+	// Extract role_id from the role information
+	roleID, ok := roleInfo["role_id"].(string)
+	if !ok || roleID == "" {
+		return "", fmt.Errorf("user %s has no role_id assigned", userID)
+	}
+
+	return roleID, nil
+}
+
+// getTeamRole gets the role for a team from database
+func (m *Manager) getTeamRole(ctx context.Context, teamID string) (string, error) {
+	if m.provider == nil {
+		return "", fmt.Errorf("user provider is not configured")
+	}
+
+	// Get team information
+	teamInfo, err := m.provider.GetTeam(ctx, teamID)
+	if err != nil {
+		return "", fmt.Errorf("failed to get team: %w", err)
+	}
+
+	// Extract role_id from the team information
+	// Note: Teams might not have a role_id field, adjust based on your schema
+	roleID, ok := teamInfo["role_id"].(string)
+	if !ok || roleID == "" {
+		// If team doesn't have a role, return a default team role
+		return "team:default", nil
+	}
+
+	return roleID, nil
+}
+
+// getMemberRole gets the role for a team member from database
+func (m *Manager) getMemberRole(ctx context.Context, teamID, userID string) (string, error) {
+	if m.provider == nil {
+		return "", fmt.Errorf("user provider is not configured")
+	}
+
+	// Get member information
+	memberInfo, err := m.provider.GetMember(ctx, teamID, userID)
+	if err != nil {
+		return "", fmt.Errorf("failed to get member: %w", err)
+	}
+
+	// Extract role_id from the member information
+	roleID, ok := memberInfo["role_id"].(string)
+	if !ok || roleID == "" {
+		return "", fmt.Errorf("member %s in team %s has no role_id assigned", userID, teamID)
+	}
+
+	return roleID, nil
+}
+
+// getScopes gets the scopes for a role from database
+// Returns: (allowedScopes, restrictedScopes, error)
+func (m *Manager) getScopes(ctx context.Context, roleID string) ([]string, []string, error) {
+	if m.provider == nil {
+		return nil, nil, fmt.Errorf("user provider is not configured")
+	}
+
+	// Get role permissions which should contain scopes
+	permissionsData, err := m.provider.GetRolePermissions(ctx, roleID)
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed to get role permissions: %w", err)
+	}
+
+	// Extract allowed scopes (positive permissions)
+	allowedScopes := []string{}
+	if permissionsInterface, ok := permissionsData["permissions"]; ok {
+		allowed, err := formatPermissions(permissionsInterface)
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to format permissions: %w", err)
+		}
+		allowedScopes = allowed
+	}
+
+	// Extract restricted scopes (negative permissions)
+	restrictedScopes := []string{}
+	if restrictedInterface, ok := permissionsData["restricted_permissions"]; ok {
+		restricted, err := formatPermissions(restrictedInterface)
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to format restricted_permissions: %w", err)
+		}
+		restrictedScopes = restricted
+	}
+
+	return allowedScopes, restrictedScopes, nil
+}

openapi/oauth/acl/role/types.go

@@ -0,0 +1,12 @@
+package role
+
+import (
+	"github.com/yaoapp/gou/store"
+	"github.com/yaoapp/yao/openapi/oauth/types"
+)
+
+// Manager is the role manager
+type Manager struct {
+	cache    store.Store
+	provider types.UserProvider
+}

openapi/oauth/acl/role/utils.go

@@ -0,0 +1,129 @@
+package role
+
+import "fmt"
+
+// ============================================================================
+// Type Conversion Utilities
+// ============================================================================
+
+// toString converts the value to a string
+func toString(value interface{}) string {
+	switch v := value.(type) {
+	case string:
+		return v
+	case []byte:
+		return string(v)
+	default:
+		return fmt.Sprintf("%v", v)
+	}
+}
+
+// toStringArray converts various types to a string slice
+func toStringArray(value interface{}) []string {
+	switch v := value.(type) {
+	case []string:
+		return v
+	case []interface{}:
+		result := []string{}
+		for _, v := range v {
+			result = append(result, toString(v))
+		}
+		return result
+	default:
+		return []string{}
+	}
+}
+
+// ============================================================================
+// Permission Format Utilities
+// ============================================================================
+
+// formatPermissions converts various permission formats to a string slice
+// Supports: []string, []interface{}, map[string]interface{}, map[string]bool, string
+func formatPermissions(value interface{}) ([]string, error) {
+	if value == nil {
+		return []string{}, nil
+	}
+
+	switch v := value.(type) {
+	case []string:
+		// Direct string slice
+		return v, nil
+
+	case []interface{}:
+		// Interface slice - convert each element
+		result := make([]string, 0, len(v))
+		for i, item := range v {
+			switch itemVal := item.(type) {
+			case string:
+				result = append(result, itemVal)
+			case []byte:
+				result = append(result, string(itemVal))
+			default:
+				return nil, fmt.Errorf("item at index %d has unsupported type %T", i, item)
+			}
+		}
+		return result, nil
+
+	case map[string]interface{}:
+		// Map with interface{} values - extract keys where value is truthy
+		result := make([]string, 0, len(v))
+		for key, val := range v {
+			// Include if value is truthy
+			if isTrue(val) {
+				result = append(result, key)
+			}
+		}
+		return result, nil
+
+	case map[string]bool:
+		// Map with bool values - extract keys where value is true
+		result := make([]string, 0, len(v))
+		for key, enabled := range v {
+			if enabled {
+				result = append(result, key)
+			}
+		}
+		return result, nil
+
+	case string:
+		// Single string - return as single-element slice
+		if v == "" {
+			return []string{}, nil
+		}
+		return []string{v}, nil
+
+	case []byte:
+		// Byte slice - convert to string
+		str := string(v)
+		if str == "" {
+			return []string{}, nil
+		}
+		return []string{str}, nil
+
+	default:
+		return nil, fmt.Errorf("unsupported permissions type: %T", value)
+	}
+}
+
+// isTrue checks if a value is truthy
+func isTrue(value interface{}) bool {
+	if value == nil {
+		return false
+	}
+
+	switch v := value.(type) {
+	case bool:
+		return v
+	case int, int8, int16, int32, int64:
+		return v != 0
+	case uint, uint8, uint16, uint32, uint64:
+		return v != 0
+	case float32, float64:
+		return v != 0
+	case string:
+		return v != "" && v != "false" && v != "0"
+	default:
+		return true // Non-nil, non-false values are considered truthy
+	}
+}

openapi/oauth/acl/types.go

@@ -4,6 +4,9 @@ import (
 	"fmt"
 	"strings"
 	"sync"
+
+	"github.com/yaoapp/gou/store"
+	"github.com/yaoapp/yao/openapi/oauth/types"
 )
 
 // DefaultConfig is the default configuration for the ACL
@@ -13,7 +16,9 @@ var DefaultConfig = Config{
 
 // Config is the configuration for the ACL
 type Config struct {
-	Enabled bool `json:"enabled"`
+	Enabled  bool               `json:"enabled"`
+	Cache    store.Store        `json:"-"`
+	Provider types.UserProvider `json:"-"`
 }
 
 // ACL is the ACL checker

openapi/openapi.go

@@ -65,7 +65,7 @@ func Load(appConfig config.Config) (*OpenAPI, error) {
 	}
 
 	// Load the ACL enforcer
-	_, err = acl.Load(&acl.Config{Enabled: true})
+	_, err = acl.Load(&acl.Config{Enabled: true, Cache: oauthConfig.Cache, Provider: oauthConfig.UserProvider})
 	if err != nil {
 		return nil, err
 	}

openapi/tests/oauth/acl/enforce_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/yaoapp/yao/openapi/oauth/acl"
 	"github.com/yaoapp/yao/openapi/oauth/authorized"
 	"github.com/yaoapp/yao/openapi/oauth/types"
+	"github.com/yaoapp/yao/openapi/tests/testutils"
 )
 
 // setupGinContext creates a test gin context with authorized info
@@ -89,13 +90,16 @@ func TestEnforce(t *testing.T) {
 		}
 
 		// Setup test context with no scopes
-		c, w := setupGinContext("GET", "/test/endpoint", []string{})
+		c, _ := setupGinContext("GET", "/test/endpoint", []string{})
 
-		// Enforce should check permissions
+		// Enforce should deny access and return error (no scope for unmatched endpoint)
 		allowed, err := aclEnforcer.Enforce(c)
-		assert.NoError(t, err)
+		assert.False(t, allowed, "Should deny access for unmatched endpoint")
+		assert.Error(t, err, "Should return error when access is denied")
 
-		t.Logf("Access decision: allowed=%v, status=%d", allowed, w.Code)
+		if err != nil {
+			t.Logf("Access correctly denied: %v", err)
+		}
 	})
 
 	t.Run("EnforceWithScopes", func(t *testing.T) {
@@ -111,18 +115,15 @@ func TestEnforce(t *testing.T) {
 			return
 		}
 
-		// Setup test context with scopes
-		c, w := setupGinContext("GET", "/api/users", []string{"read:users", "write:users"})
+		// Setup test context with scopes for unmatched endpoint
+		c, _ := setupGinContext("GET", "/api/users", []string{"read:users", "write:users"})
 
-		// Enforce should check permissions
+		// Enforce should deny (unmatched endpoint, default policy: deny)
 		allowed, err := aclEnforcer.Enforce(c)
-		assert.NoError(t, err)
+		assert.False(t, allowed, "Should deny access for unmatched endpoint")
+		assert.Error(t, err, "Should return error when access is denied")
 
-		t.Logf("Access with scopes: allowed=%v, status=%d", allowed, w.Code)
-
-		if !allowed && w.Code == 403 {
-			t.Log("Access correctly denied with 403 response")
-		}
+		t.Logf("Access correctly denied: %v", err)
 	})
 
 	t.Run("EnforceChecksContext", func(t *testing.T) {
@@ -151,17 +152,19 @@ func TestEnforce(t *testing.T) {
 		assert.Equal(t, "test-client", authInfo.ClientID)
 		assert.Contains(t, authInfo.Scope, "collections:create")
 
-		// Enforce
+		// Enforce - should deny because required scope is "collections:write:all"
 		allowed, err := aclEnforcer.Enforce(c)
-		assert.NoError(t, err)
+		assert.False(t, allowed, "Should deny access without required scope")
+		assert.Error(t, err, "Should return error for missing required scopes")
 
-		t.Logf("Enforce with collections scopes: allowed=%v", allowed)
+		t.Logf("Access correctly denied: %v", err)
 	})
 }
 
-// TestEnforceResponseFormat tests the response format when access is denied
-func TestEnforceResponseFormat(t *testing.T) {
-	t.Run("DeniedAccessResponse", func(t *testing.T) {
+// TestEnforceReturnValues tests Enforce return values when access is denied
+// Note: HTTP response format is handled by Guard middleware, not by Enforce
+func TestEnforceReturnValues(t *testing.T) {
+	t.Run("DeniedAccessReturnValues", func(t *testing.T) {
 		config := &acl.Config{
 			Enabled: true,
 		}
@@ -173,28 +176,22 @@ func TestEnforceResponseFormat(t *testing.T) {
 			return
 		}
 
-		// Setup context with insufficient scopes for a protected endpoint
-		c, w := setupGinContext("POST", "/protected/admin", []string{"read:basic"})
+		// Setup context with scopes for an unmatched endpoint
+		c, _ := setupGinContext("POST", "/protected/admin", []string{"read:basic"})
 
-		// Enforce
+		// Enforce should return false and an error
 		allowed, err := aclEnforcer.Enforce(c)
-		assert.NoError(t, err)
+		assert.False(t, allowed, "Should deny access")
+		assert.Error(t, err, "Should return error when access is denied")
 
-		// If access is denied, check response format
-		if !allowed {
-			assert.Equal(t, 403, w.Code, "Should return 403 Forbidden")
-
-			// Response should be JSON
-			contentType := w.Header().Get("Content-Type")
-			assert.Contains(t, contentType, "application/json")
-
-			// Response body should contain error details
-			body := w.Body.String()
-			assert.Contains(t, body, "Access denied")
-
-			t.Logf("Denied access response format correct: %s", body)
-		} else {
-			t.Log("Access was allowed (no scope configuration for this endpoint)")
+		// Verify error contains ACL error information
+		if err != nil {
+			aclErr, ok := err.(*acl.Error)
+			assert.True(t, ok, "Error should be ACL Error type")
+			if aclErr != nil {
+				assert.NotEmpty(t, aclErr.Message, "Error should have message")
+				t.Logf("Access correctly denied with error: %v", aclErr.Message)
+			}
 		}
 	})
 }
@@ -246,6 +243,9 @@ func TestGetScopes(t *testing.T) {
 
 // TestEnforceIntegration tests the complete enforcement flow
 func TestEnforceIntegration(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
 	t.Run("CompleteFlow", func(t *testing.T) {
 		// Create enabled ACL
 		config := &acl.Config{
@@ -312,9 +312,9 @@ func TestEnforceIntegration(t *testing.T) {
 			{
 				name:     "WildcardAllowedRead",
 				method:   "GET",
-				path:     "/kb/documents/doc-123",
+				path:     "/kb/some-other-resource/item-123",
 				scopes:   []string{},
-				expected: "allow", // GET /kb/* allow from scopes.yml
+				expected: "allow", // GET /kb/* allow from scopes.yml (wildcard match, no specific scope defined)
 			},
 			{
 				name:     "UnmatchedEndpoint",
@@ -327,17 +327,20 @@ func TestEnforceIntegration(t *testing.T) {
 
 		for _, tc := range testCases {
 			t.Run(tc.name, func(t *testing.T) {
-				c, w := setupGinContext(tc.method, tc.path, tc.scopes)
+				c, _ := setupGinContext(tc.method, tc.path, tc.scopes)
 
 				allowed, err := aclEnforcer.Enforce(c)
-				assert.NoError(t, err)
 
-				t.Logf("%s %s with scopes %v: allowed=%v, status=%d",
-					tc.method, tc.path, tc.scopes, allowed, w.Code)
+				t.Logf("%s %s with scopes %v: allowed=%v",
+					tc.method, tc.path, tc.scopes, allowed)
 
-				// Verify response is properly formatted
-				if !allowed {
-					assert.Equal(t, 403, w.Code)
+				// Verify Enforce return values (not HTTP response format)
+				if tc.expected == "allow" {
+					assert.True(t, allowed, "Should allow access")
+					assert.NoError(t, err, "Should not return error when access is allowed")
+				} else {
+					assert.False(t, allowed, "Should deny access")
+					assert.Error(t, err, "Should return error when access is denied")
 				}
 			})
 		}
@@ -395,13 +398,14 @@ func TestEnforceEdgeCases(t *testing.T) {
 			return
 		}
 
-		// Test with special characters in path
+		// Test with special characters in path (unmatched endpoint)
 		c, _ := setupGinContext("GET", "/api/users/%20with%20spaces", []string{"read:users"})
 
 		allowed, err := aclEnforcer.Enforce(c)
-		assert.NoError(t, err)
+		assert.False(t, allowed, "Should deny unmatched endpoint")
+		assert.Error(t, err, "Should return error for unmatched endpoint")
 
-		t.Logf("Path with special chars: allowed=%v", allowed)
+		t.Logf("Path with special chars correctly denied: %v", err)
 	})
 
 	t.Run("VeryLongScope", func(t *testing.T) {
@@ -416,13 +420,14 @@ func TestEnforceEdgeCases(t *testing.T) {
 			return
 		}
 
-		// Test with very long scope name
+		// Test with very long scope name (unmatched endpoint)
 		longScope := "very:long:scope:name:with:many:segments:to:test:handling:of:long:strings"
 		c, _ := setupGinContext("GET", "/test", []string{longScope})
 
 		allowed, err := aclEnforcer.Enforce(c)
-		assert.NoError(t, err)
+		assert.False(t, allowed, "Should deny unmatched endpoint")
+		assert.Error(t, err, "Should return error for unmatched endpoint")
 
-		t.Logf("Long scope handling: allowed=%v", allowed)
+		t.Logf("Long scope correctly handled: %v", err)
 	})
 }

openapi/tests/oauth/acl/role/role_test.go

@@ -0,0 +1,538 @@
+package role_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/yaoapp/kun/maps"
+	"github.com/yaoapp/yao/openapi/oauth"
+	"github.com/yaoapp/yao/openapi/oauth/acl/role"
+	"github.com/yaoapp/yao/openapi/tests/testutils"
+)
+
+// Role Manager Test Suite
+//
+// PREREQUISITES:
+// Before running tests, source the environment file:
+//   source $YAO_DEV/env.local.sh
+//
+// Then run tests:
+//   go test -v ./openapi/tests/oauth/acl/role/... -count=1
+
+func TestNewManager(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
+	t.Run("CreateManagerWithProvider", func(t *testing.T) {
+		// Get cache and provider from OAuth service
+		cache := oauth.OAuth.GetCache()
+		require.NotNil(t, cache)
+
+		provider, err := oauth.OAuth.GetUserProvider()
+		require.NoError(t, err)
+		require.NotNil(t, provider)
+
+		// Create manager
+		manager := role.NewManager(cache, provider)
+		assert.NotNil(t, manager)
+
+		t.Log("Successfully created role manager with cache and provider")
+	})
+
+	t.Run("CreateManagerWithNilProvider", func(t *testing.T) {
+		// Get cache from OAuth service
+		cache := oauth.OAuth.GetCache()
+		require.NotNil(t, cache)
+
+		// Create manager with nil provider
+		manager := role.NewManager(cache, nil)
+		assert.NotNil(t, manager)
+
+		// Should work but will error when trying to get roles
+		ctx := context.Background()
+		_, err := manager.GetUserRole(ctx, "test-user")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "user provider is not configured")
+
+		t.Log("Manager with nil provider correctly returns error")
+	})
+}
+
+func TestManagerWithNilCache(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
+	// Get provider
+	provider, err := oauth.OAuth.GetUserProvider()
+	require.NoError(t, err)
+	require.NotNil(t, provider)
+
+	// Create manager with nil cache
+	manager := role.NewManager(nil, provider)
+	require.NotNil(t, manager)
+
+	ctx := context.Background()
+
+	t.Run("GetUserRoleWithNilCache", func(t *testing.T) {
+		// Create a test role and user
+		testRoleID := "test_role_nil_cache"
+		roleData := maps.MapStrAny{
+			"role_id":     testRoleID,
+			"name":        "Test Nil Cache Role",
+			"description": "Role for nil cache testing",
+			"is_active":   true,
+		}
+		_, err := provider.CreateRole(ctx, roleData)
+		if err != nil {
+			t.Skipf("Skipping test - cannot create role: %v", err)
+			return
+		}
+		defer provider.DeleteRole(ctx, testRoleID)
+
+		userID, err := provider.GenerateUserID(ctx, true)
+		require.NoError(t, err)
+
+		userData := maps.MapStrAny{
+			"user_id": userID,
+			"email":   "nilcache@example.com",
+			"role_id": testRoleID,
+			"status":  "active",
+		}
+		_, err = provider.CreateUser(ctx, userData)
+		require.NoError(t, err)
+		defer provider.DeleteUser(ctx, userID)
+
+		// Get user role - should work even without cache
+		roleID, err := manager.GetUserRole(ctx, userID)
+		assert.NoError(t, err)
+		assert.Equal(t, testRoleID, roleID)
+
+		t.Log("Successfully retrieved role without cache")
+	})
+
+	t.Run("GetScopesWithNilCache", func(t *testing.T) {
+		// Create test role with permissions
+		testRoleID := "test_role_scopes_nil_cache"
+		permissions := maps.MapStrAny{
+			"read":  true,
+			"write": true,
+		}
+		restrictedPermissions := []string{"admin"}
+
+		roleData := maps.MapStrAny{
+			"role_id":                testRoleID,
+			"name":                   "Test Nil Cache Scopes",
+			"description":            "Role for scopes nil cache testing",
+			"is_active":              true,
+			"permissions":            permissions,
+			"restricted_permissions": restrictedPermissions,
+		}
+		_, err := provider.CreateRole(ctx, roleData)
+		if err != nil {
+			t.Skipf("Skipping test - cannot create role: %v", err)
+			return
+		}
+		defer provider.DeleteRole(ctx, testRoleID)
+
+		// Get scopes - should work even without cache
+		allowed, restricted, err := manager.GetScopes(ctx, testRoleID)
+		assert.NoError(t, err)
+		assert.NotNil(t, allowed)
+		assert.NotNil(t, restricted)
+
+		t.Logf("Successfully retrieved scopes without cache: allowed=%v, restricted=%v", allowed, restricted)
+	})
+
+	t.Run("ClearCacheWithNilCache", func(t *testing.T) {
+		// Should not panic with nil cache
+		err := manager.ClearCache()
+		assert.NoError(t, err)
+
+		t.Log("ClearCache works safely with nil cache")
+	})
+}
+
+func TestGetUserRole(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
+	// Setup
+	cache := oauth.OAuth.GetCache()
+	require.NotNil(t, cache)
+
+	provider, err := oauth.OAuth.GetUserProvider()
+	require.NoError(t, err)
+	require.NotNil(t, provider)
+
+	manager := role.NewManager(cache, provider)
+	ctx := context.Background()
+
+	t.Run("GetRoleForExistingUser", func(t *testing.T) {
+		// First, create a test role using provider
+		testRoleID := "test_role_get_user"
+		roleData := maps.MapStrAny{
+			"role_id":     testRoleID,
+			"name":        "Test Role",
+			"description": "Role for testing",
+			"is_active":   true,
+		}
+
+		// Create role
+		createdRoleID, err := provider.CreateRole(ctx, roleData)
+		if err != nil {
+			t.Skipf("Skipping test - cannot create role: %v", err)
+			return
+		}
+		require.NotEmpty(t, createdRoleID)
+		defer provider.DeleteRole(ctx, testRoleID)
+
+		// Generate unique user ID
+		userID, err := provider.GenerateUserID(ctx, true)
+		require.NoError(t, err)
+
+		// Create test user with role
+		userData := maps.MapStrAny{
+			"user_id": userID,
+			"email":   "testuser@example.com",
+			"role_id": testRoleID,
+			"status":  "active",
+		}
+		_, err = provider.CreateUser(ctx, userData)
+		require.NoError(t, err)
+		defer provider.DeleteUser(ctx, userID)
+
+		// Get user role
+		roleID, err := manager.GetUserRole(ctx, userID)
+		assert.NoError(t, err)
+		assert.Equal(t, testRoleID, roleID)
+
+		t.Logf("Successfully retrieved role %s for user %s", roleID, userID)
+
+		// Get again (should come from cache)
+		roleID2, err := manager.GetUserRole(ctx, userID)
+		assert.NoError(t, err)
+		assert.Equal(t, testRoleID, roleID2)
+
+		t.Log("Successfully retrieved role from cache")
+	})
+
+	t.Run("GetRoleForNonExistentUser", func(t *testing.T) {
+		_, err := manager.GetUserRole(ctx, "non-existent-user-12345")
+		assert.Error(t, err)
+
+		t.Log("Correctly returns error for non-existent user")
+	})
+
+	t.Run("GetRoleForUserWithoutRole", func(t *testing.T) {
+		// Create user without role
+		userID, err := provider.GenerateUserID(ctx, true)
+		require.NoError(t, err)
+
+		userData := maps.MapStrAny{
+			"user_id": userID,
+			"email":   "norole@example.com",
+			"status":  "active",
+		}
+		_, err = provider.CreateUser(ctx, userData)
+		require.NoError(t, err)
+		defer provider.DeleteUser(ctx, userID)
+
+		_, err = manager.GetUserRole(ctx, userID)
+		assert.Error(t, err)
+
+		t.Log("Correctly returns error for user without role")
+	})
+}
+
+func TestGetMemberRole(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
+	// Setup
+	cache := oauth.OAuth.GetCache()
+	require.NotNil(t, cache)
+
+	provider, err := oauth.OAuth.GetUserProvider()
+	require.NoError(t, err)
+	require.NotNil(t, provider)
+
+	manager := role.NewManager(cache, provider)
+	ctx := context.Background()
+
+	t.Run("GetRoleForExistingMember", func(t *testing.T) {
+		// Create test role
+		testRoleID := "test_role_member"
+		roleData := maps.MapStrAny{
+			"role_id":     testRoleID,
+			"name":        "Test Member Role",
+			"description": "Role for member testing",
+			"is_active":   true,
+		}
+		_, err := provider.CreateRole(ctx, roleData)
+		if err != nil {
+			t.Skipf("Skipping test - cannot create role: %v", err)
+			return
+		}
+		defer provider.DeleteRole(ctx, testRoleID)
+
+		// Create test team
+		teamID, err := provider.GenerateUserID(ctx, true)
+		require.NoError(t, err)
+
+		teamData := maps.MapStrAny{
+			"team_id":  teamID,
+			"name":     "Test Team",
+			"owner_id": "test_owner",
+			"status":   "active",
+		}
+		_, err = provider.CreateTeam(ctx, teamData)
+		require.NoError(t, err)
+		defer provider.DeleteTeam(ctx, teamID)
+
+		// Create test member
+		userID, err := provider.GenerateUserID(ctx, true)
+		require.NoError(t, err)
+
+		memberData := maps.MapStrAny{
+			"team_id":     teamID,
+			"user_id":     userID,
+			"role_id":     testRoleID,
+			"member_type": "user",
+			"status":      "active",
+		}
+		_, err = provider.CreateMember(ctx, memberData)
+		require.NoError(t, err)
+		defer provider.RemoveMember(ctx, teamID, userID)
+
+		// Get member role
+		roleID, err := manager.GetMemberRole(ctx, teamID, userID)
+		assert.NoError(t, err)
+		assert.Equal(t, testRoleID, roleID)
+
+		t.Logf("Successfully retrieved role %s for member %s in team %s", roleID, userID, teamID)
+
+		// Get again (should come from cache)
+		roleID2, err := manager.GetMemberRole(ctx, teamID, userID)
+		assert.NoError(t, err)
+		assert.Equal(t, testRoleID, roleID2)
+
+		t.Log("Successfully retrieved member role from cache")
+	})
+
+	t.Run("GetRoleForNonExistentMember", func(t *testing.T) {
+		_, err := manager.GetMemberRole(ctx, "non-existent-team", "non-existent-user")
+		assert.Error(t, err)
+
+		t.Log("Correctly returns error for non-existent member")
+	})
+}
+
+func TestGetScopes(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
+	// Setup
+	cache := oauth.OAuth.GetCache()
+	require.NotNil(t, cache)
+
+	provider, err := oauth.OAuth.GetUserProvider()
+	require.NoError(t, err)
+	require.NotNil(t, provider)
+
+	manager := role.NewManager(cache, provider)
+	ctx := context.Background()
+
+	t.Run("GetScopesForRoleWithPermissions", func(t *testing.T) {
+		// Create test role with permissions
+		testRoleID := "test_role_scopes"
+
+		// Create role with permissions as map
+		permissions := maps.MapStrAny{
+			"read":   true,
+			"write":  true,
+			"delete": false, // Should not be included
+		}
+
+		restrictedPermissions := []string{"admin", "superuser"}
+
+		roleData := maps.MapStrAny{
+			"role_id":                testRoleID,
+			"name":                   "Test Role with Scopes",
+			"description":            "Role for scopes testing",
+			"is_active":              true,
+			"permissions":            permissions,
+			"restricted_permissions": restrictedPermissions,
+		}
+		_, err := provider.CreateRole(ctx, roleData)
+		if err != nil {
+			t.Skipf("Skipping test - cannot create role: %v", err)
+			return
+		}
+		defer provider.DeleteRole(ctx, testRoleID)
+
+		// Get scopes
+		allowed, restricted, err := manager.GetScopes(ctx, testRoleID)
+		assert.NoError(t, err)
+		assert.NotNil(t, allowed)
+		assert.NotNil(t, restricted)
+
+		// Verify allowed scopes contain enabled permissions
+		t.Logf("Allowed scopes: %v", allowed)
+		t.Logf("Restricted scopes: %v", restricted)
+
+		// Note: The exact format depends on how the database stores JSON
+		// We just verify we can retrieve them without error
+		assert.True(t, len(allowed) >= 0, "Should return allowed scopes (empty or with values)")
+		assert.True(t, len(restricted) >= 0, "Should return restricted scopes (empty or with values)")
+
+		// Get again (should come from cache)
+		allowed2, restricted2, err := manager.GetScopes(ctx, testRoleID)
+		assert.NoError(t, err)
+		assert.Equal(t, allowed, allowed2)
+		assert.Equal(t, restricted, restricted2)
+
+		t.Log("Successfully retrieved scopes from cache")
+	})
+
+	t.Run("GetScopesForNonExistentRole", func(t *testing.T) {
+		_, _, err := manager.GetScopes(ctx, "non-existent-role-12345")
+		assert.Error(t, err)
+
+		t.Log("Correctly returns error for non-existent role")
+	})
+}
+
+func TestGetTeamRole(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
+	// Setup
+	cache := oauth.OAuth.GetCache()
+	require.NotNil(t, cache)
+
+	provider, err := oauth.OAuth.GetUserProvider()
+	require.NoError(t, err)
+	require.NotNil(t, provider)
+
+	manager := role.NewManager(cache, provider)
+	ctx := context.Background()
+
+	t.Run("GetRoleForTeamWithoutRole", func(t *testing.T) {
+		// Create team without role_id field
+		teamID, err := provider.GenerateUserID(ctx, true)
+		require.NoError(t, err)
+
+		teamData := maps.MapStrAny{
+			"team_id":  teamID,
+			"name":     "Test Team No Role",
+			"owner_id": "test_owner",
+			"status":   "active",
+		}
+		_, err = provider.CreateTeam(ctx, teamData)
+		require.NoError(t, err)
+		defer provider.DeleteTeam(ctx, teamID)
+
+		// Get team role (should return default)
+		roleID, err := manager.GetTeamRole(ctx, teamID)
+		assert.NoError(t, err)
+		assert.Equal(t, "team:default", roleID)
+
+		t.Logf("Successfully returned default role for team without role_id: %s", roleID)
+	})
+
+	t.Run("GetRoleForNonExistentTeam", func(t *testing.T) {
+		_, err := manager.GetTeamRole(ctx, "non-existent-team-12345")
+		assert.Error(t, err)
+
+		t.Log("Correctly returns error for non-existent team")
+	})
+}
+
+func TestGetClientRole(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
+	// Setup
+	cache := oauth.OAuth.GetCache()
+	require.NotNil(t, cache)
+
+	provider, err := oauth.OAuth.GetUserProvider()
+	require.NoError(t, err)
+	require.NotNil(t, provider)
+
+	manager := role.NewManager(cache, provider)
+	ctx := context.Background()
+
+	t.Run("GetClientRoleReturnsDefault", func(t *testing.T) {
+		// Note: Client role retrieval is TODO in the code
+		// It currently returns a default "system:root" role
+		roleID, err := manager.GetClientRole(ctx, "test-client")
+		assert.NoError(t, err)
+		assert.Equal(t, "system:root", roleID)
+
+		t.Log("Client role returns default system:root (TODO: implement ClientProvider)")
+	})
+}
+
+func TestCacheIntegration(t *testing.T) {
+	testutils.Prepare(t)
+	defer testutils.Clean()
+
+	// Setup
+	cache := oauth.OAuth.GetCache()
+	require.NotNil(t, cache)
+
+	provider, err := oauth.OAuth.GetUserProvider()
+	require.NoError(t, err)
+	require.NotNil(t, provider)
+
+	manager := role.NewManager(cache, provider)
+	ctx := context.Background()
+
+	t.Run("RoleCachingWorks", func(t *testing.T) {
+		// Create test role and user
+		testRoleID := "test_role_cache"
+		roleData := maps.MapStrAny{
+			"role_id":     testRoleID,
+			"name":        "Test Cache Role",
+			"description": "Role for cache testing",
+			"is_active":   true,
+		}
+		_, err := provider.CreateRole(ctx, roleData)
+		if err != nil {
+			t.Skipf("Skipping test - cannot create role: %v", err)
+			return
+		}
+		defer provider.DeleteRole(ctx, testRoleID)
+
+		userID, err := provider.GenerateUserID(ctx, true)
+		require.NoError(t, err)
+
+		userData := maps.MapStrAny{
+			"user_id": userID,
+			"email":   "cache@example.com",
+			"role_id": testRoleID,
+			"status":  "active",
+		}
+		_, err = provider.CreateUser(ctx, userData)
+		require.NoError(t, err)
+		defer provider.DeleteUser(ctx, userID)
+
+		// First call - should hit database
+		roleID1, err := manager.GetUserRole(ctx, userID)
+		assert.NoError(t, err)
+		assert.Equal(t, testRoleID, roleID1)
+
+		// Second call - should hit cache
+		roleID2, err := manager.GetUserRole(ctx, userID)
+		assert.NoError(t, err)
+		assert.Equal(t, testRoleID, roleID2)
+
+		// Results should be identical
+		assert.Equal(t, roleID1, roleID2)
+
+		t.Log("Cache integration verified: same role retrieved from cache")
+	})
+}