From 1b4f78af6dd31e43dbf14ac2772d3529ea5a15eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 8 Jan 2026 17:54:51 +0000 Subject: [PATCH 1/7] Initial plan From da35b4338bcc505b60853df47ec2f113419dfee7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 8 Jan 2026 18:02:32 +0000 Subject: [PATCH 2/7] Add RateLimitEffector with comprehensive tests and integration Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- effector/rate_limit_effector.go | 227 ++++++++++++++++ effector/rate_limit_effector_test.go | 393 +++++++++++++++++++++++++++ enforcer.go | 28 ++ examples/rate_limit_model.conf | 11 + examples/rate_limit_policy.csv | 4 + rate_limit_test.go | 251 +++++++++++++++++ 6 files changed, 914 insertions(+) create mode 100644 effector/rate_limit_effector.go create mode 100644 effector/rate_limit_effector_test.go create mode 100644 examples/rate_limit_model.conf create mode 100644 examples/rate_limit_policy.csv create mode 100644 rate_limit_test.go diff --git a/effector/rate_limit_effector.go b/effector/rate_limit_effector.go new file mode 100644 index 000000000..0d730d1a6 --- /dev/null +++ b/effector/rate_limit_effector.go @@ -0,0 +1,227 @@ +// Copyright 2026 The casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package effector + +import ( + "errors" + "fmt" + "regexp" + "strconv" + "strings" + "sync" + "time" +) + +// RateLimitBucket holds the state for a rate limit bucket +type RateLimitBucket struct { + count int + windowEnd time.Time +} + +// RateLimitEffector is an effector that implements rate limiting +type RateLimitEffector struct { + mu sync.RWMutex + buckets map[string]*RateLimitBucket + requestContext map[string]string // stores current request context (sub, obj, act) +} + +// NewRateLimitEffector creates a new RateLimitEffector +func NewRateLimitEffector() *RateLimitEffector { + return &RateLimitEffector{ + buckets: make(map[string]*RateLimitBucket), + requestContext: make(map[string]string), + } +} + +var rateLimitRegex = regexp.MustCompile(`rate_limit\((\d+),\s*(\w+),\s*(\w+),\s*(\w+)\)`) + +// parseRateLimitExpr parses a rate_limit expression +// Format: rate_limit(max, unit, count_type, bucket) +func parseRateLimitExpr(expr string) (max int, unit string, countType string, bucket string, err error) { + matches := rateLimitRegex.FindStringSubmatch(expr) + if matches == nil || len(matches) != 5 { + return 0, "", "", "", fmt.Errorf("invalid rate_limit expression: %s", expr) + } + + max, err = strconv.Atoi(matches[1]) + if err != nil { + return 0, "", "", "", fmt.Errorf("invalid max value: %s", matches[1]) + } + + unit = matches[2] + countType = matches[3] + bucket = matches[4] + + // Validate unit + validUnits := map[string]bool{"second": true, "minute": true, "hour": true, "day": true} + if !validUnits[unit] { + return 0, "", "", "", fmt.Errorf("invalid unit: %s (must be second, minute, hour, or day)", unit) + } + + // Validate count_type + if countType != "allow" && countType != "all" { + return 0, "", "", "", fmt.Errorf("invalid count_type: %s (must be allow or all)", countType) + } + + // Validate bucket + validBuckets := map[string]bool{"all": true, "sub": true, "obj": true, "act": true} + if !validBuckets[bucket] { + return 0, "", "", "", fmt.Errorf("invalid bucket: %s (must be all, sub, obj, or act)", bucket) + } + + return max, unit, countType, bucket, nil +} + +// getWindowDuration returns the duration for a time unit +func getWindowDuration(unit string) time.Duration { + switch unit { + case "second": + return time.Second + case "minute": + return time.Minute + case "hour": + return time.Hour + case "day": + return 24 * time.Hour + default: + return time.Minute + } +} + +// MergeEffects implements the Effector interface with rate limiting +func (e *RateLimitEffector) MergeEffects(expr string, effects []Effect, matches []float64, policyIndex int, policyLength int) (Effect, int, error) { + // Check if this is a rate_limit expression + if !strings.Contains(expr, "rate_limit") { + return Deny, -1, errors.New("RateLimitEffector requires rate_limit expression") + } + + max, unit, countType, bucketType, err := parseRateLimitExpr(expr) + if err != nil { + return Deny, -1, err + } + + // For rate limiting, we need to check all matches first to determine the base result + var baseEffect Effect = Indeterminate + var explainIndex int = -1 + + // Find the first matching policy + for i := 0; i < policyLength; i++ { + if matches[i] != 0 { + if effects[i] == Allow { + baseEffect = Allow + explainIndex = i + break + } else if effects[i] == Deny { + baseEffect = Deny + explainIndex = i + break + } + } + } + + // Count this request if needed + shouldCount := false + if countType == "all" { + // Count all requests + shouldCount = true + } else if countType == "allow" && baseEffect == Allow { + // Only count allowed requests + shouldCount = true + } + + // Generate bucket key based on bucket type and request context + bucketKey := e.generateBucketKey(bucketType) + + // Check and update rate limit + if shouldCount { + now := time.Now() + windowDuration := getWindowDuration(unit) + + e.mu.Lock() + defer e.mu.Unlock() + + bucket, exists := e.buckets[bucketKey] + if !exists || now.After(bucket.windowEnd) { + // Create new window + e.buckets[bucketKey] = &RateLimitBucket{ + count: 1, + windowEnd: now.Add(windowDuration), + } + } else { + // Increment counter in current window + bucket.count++ + if bucket.count > max { + // Rate limit exceeded + return Deny, -1, nil + } + } + } + + return baseEffect, explainIndex, nil +} + +// SetRequestContext sets the request context for bucket key generation +// This should be called before MergeEffects to provide request context +func (e *RateLimitEffector) SetRequestContext(sub, obj, act string) { + e.mu.Lock() + defer e.mu.Unlock() + e.requestContext["sub"] = sub + e.requestContext["obj"] = obj + e.requestContext["act"] = act +} + +// generateBucketKey generates a bucket key based on the bucket type and request context +func (e *RateLimitEffector) generateBucketKey(bucketType string) string { + switch bucketType { + case "all": + return "bucket:all" + case "sub": + if sub, ok := e.requestContext["sub"]; ok { + return fmt.Sprintf("bucket:sub:%s", sub) + } + return "bucket:sub:unknown" + case "obj": + if obj, ok := e.requestContext["obj"]; ok { + return fmt.Sprintf("bucket:obj:%s", obj) + } + return "bucket:obj:unknown" + case "act": + if act, ok := e.requestContext["act"]; ok { + return fmt.Sprintf("bucket:act:%s", act) + } + return "bucket:act:unknown" + default: + return "bucket:unknown" + } +} + +// GetBucketState returns the current state of a bucket (for testing) +func (e *RateLimitEffector) GetBucketState(key string) (count int, windowEnd time.Time, exists bool) { + e.mu.RLock() + defer e.mu.RUnlock() + + bucket, exists := e.buckets[key] + if !exists { + return 0, time.Time{}, false + } + return bucket.count, bucket.windowEnd, true +} + +// ResetBuckets clears all buckets (for testing) +func (e *RateLimitEffector) ResetBuckets() { + e.mu.Lock() + defer e.mu.Unlock() + e.buckets = make(map[string]*RateLimitBucket) +} diff --git a/effector/rate_limit_effector_test.go b/effector/rate_limit_effector_test.go new file mode 100644 index 000000000..7a1bdbc7a --- /dev/null +++ b/effector/rate_limit_effector_test.go @@ -0,0 +1,393 @@ +// Copyright 2026 The casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package effector + +import ( + "testing" + "time" +) + +func TestRateLimitEffectorBasic(t *testing.T) { + eft := NewRateLimitEffector() + + // Test rate limiting with allow count type + expr := "rate_limit(2, second, allow, sub)" + effects := []Effect{Allow} + matches := []float64{1.0} + + // First request should succeed + effect, _, err := eft.MergeEffects(expr, effects, matches, 0, 1) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Second request should succeed + effect, _, err = eft.MergeEffects(expr, effects, matches, 0, 1) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Third request should be denied (exceeds limit of 2) + effect, _, err = eft.MergeEffects(expr, effects, matches, 0, 1) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if effect != Deny { + t.Errorf("Expected Deny due to rate limit, got %v", effect) + } +} + +func TestRateLimitEffectorWindowReset(t *testing.T) { + eft := NewRateLimitEffector() + + // Use a short window for testing + expr := "rate_limit(2, second, allow, sub)" + effects := []Effect{Allow} + matches := []float64{1.0} + + // First request + effect, _, _ := eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Second request + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Third request should be denied + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Deny { + t.Errorf("Expected Deny, got %v", effect) + } + + // Wait for window to expire + time.Sleep(1100 * time.Millisecond) + + // After window reset, request should succeed again + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow after window reset, got %v", effect) + } +} + +func TestRateLimitEffectorCountTypeAll(t *testing.T) { + eft := NewRateLimitEffector() + + // Count all requests, even denied ones + expr := "rate_limit(2, second, all, sub)" + + // First request - Allow + effects := []Effect{Allow} + matches := []float64{1.0} + effect, _, _ := eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Second request - Deny (but still counts) + effects = []Effect{Deny} + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Deny { + t.Errorf("Expected Deny from policy, got %v", effect) + } + + // Third request - should be denied by rate limit even if policy allows + effects = []Effect{Allow} + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Deny { + t.Errorf("Expected Deny due to rate limit, got %v", effect) + } +} + +func TestRateLimitEffectorCountTypeAllow(t *testing.T) { + eft := NewRateLimitEffector() + + // Only count allowed requests + expr := "rate_limit(2, second, allow, sub)" + + // First request - Allow (counts) + effects := []Effect{Allow} + matches := []float64{1.0} + effect, _, _ := eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Second request - Deny (doesn't count) + effects = []Effect{Deny} + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Deny { + t.Errorf("Expected Deny from policy, got %v", effect) + } + + // Third request - Allow (counts, should be allowed as we only counted 1 so far) + effects = []Effect{Allow} + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Fourth request - should be denied by rate limit + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Deny { + t.Errorf("Expected Deny due to rate limit, got %v", effect) + } +} + +func TestRateLimitEffectorDifferentTimeUnits(t *testing.T) { + testCases := []struct { + unit string + duration time.Duration + }{ + {"second", time.Second}, + {"minute", time.Minute}, + {"hour", time.Hour}, + {"day", 24 * time.Hour}, + } + + for _, tc := range testCases { + t.Run(tc.unit, func(t *testing.T) { + eft := NewRateLimitEffector() + expr := "rate_limit(1, " + tc.unit + ", allow, sub)" + effects := []Effect{Allow} + matches := []float64{1.0} + + // First request should succeed + effect, _, err := eft.MergeEffects(expr, effects, matches, 0, 1) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Second request should be denied + effect, _, err = eft.MergeEffects(expr, effects, matches, 0, 1) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if effect != Deny { + t.Errorf("Expected Deny, got %v", effect) + } + }) + } +} + +func TestRateLimitEffectorInvalidExpressions(t *testing.T) { + eft := NewRateLimitEffector() + effects := []Effect{Allow} + matches := []float64{1.0} + + testCases := []struct { + name string + expr string + }{ + {"invalid format", "rate_limit(10)"}, + {"invalid max", "rate_limit(abc, second, allow, sub)"}, + {"invalid unit", "rate_limit(10, week, allow, sub)"}, + {"invalid count_type", "rate_limit(10, second, maybe, sub)"}, + {"invalid bucket", "rate_limit(10, second, allow, user)"}, + {"not rate_limit", "some(where (p.eft == allow))"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, _, err := eft.MergeEffects(tc.expr, effects, matches, 0, 1) + if err == nil { + t.Errorf("Expected error for invalid expression: %s", tc.expr) + } + }) + } +} + +func TestRateLimitEffectorNoMatch(t *testing.T) { + eft := NewRateLimitEffector() + + expr := "rate_limit(2, second, allow, sub)" + effects := []Effect{Allow} + matches := []float64{0.0} // No match + + // Should return Indeterminate when no policy matches + effect, explainIndex, err := eft.MergeEffects(expr, effects, matches, 0, 1) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if effect != Indeterminate { + t.Errorf("Expected Indeterminate, got %v", effect) + } + if explainIndex != -1 { + t.Errorf("Expected explainIndex -1, got %d", explainIndex) + } +} + +func TestRateLimitEffectorMultiplePolicies(t *testing.T) { + eft := NewRateLimitEffector() + + expr := "rate_limit(2, second, allow, sub)" + effects := []Effect{Deny, Allow, Allow} + matches := []float64{0.0, 1.0, 0.0} // Only second policy matches + + // First request with second policy matching + effect, explainIndex, err := eft.MergeEffects(expr, effects, matches, 1, 3) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + if explainIndex != 1 { + t.Errorf("Expected explainIndex 1, got %d", explainIndex) + } + + // Second request + effect, _, _ = eft.MergeEffects(expr, effects, matches, 1, 3) + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Third request should be denied by rate limit + effect, explainIndex, _ = eft.MergeEffects(expr, effects, matches, 1, 3) + if effect != Deny { + t.Errorf("Expected Deny, got %v", effect) + } + if explainIndex != -1 { + t.Errorf("Expected explainIndex -1 for rate limit, got %d", explainIndex) + } +} + +func TestRateLimitEffectorResetBuckets(t *testing.T) { + eft := NewRateLimitEffector() + + expr := "rate_limit(1, second, allow, sub)" + effects := []Effect{Allow} + matches := []float64{1.0} + + // First request + effect, _, _ := eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow, got %v", effect) + } + + // Second request should be denied + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Deny { + t.Errorf("Expected Deny, got %v", effect) + } + + // Reset buckets + eft.ResetBuckets() + + // After reset, request should succeed + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow after reset, got %v", effect) + } +} + +func TestRateLimitEffectorWithContext(t *testing.T) { + eft := NewRateLimitEffector() + + expr := "rate_limit(2, second, allow, sub)" + effects := []Effect{Allow} + matches := []float64{1.0} + + // Set context for alice + eft.SetRequestContext("alice", "data1", "read") + + // First two requests from alice should succeed + effect, _, _ := eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow for alice, got %v", effect) + } + + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow for alice, got %v", effect) + } + + // Third request from alice should be denied + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Deny { + t.Errorf("Expected Deny for alice, got %v", effect) + } + + // Switch context to bob - should have separate bucket + eft.SetRequestContext("bob", "data1", "read") + + // Bob's first request should succeed (separate bucket) + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow for bob, got %v", effect) + } +} + +func TestRateLimitEffectorDifferentBucketTypes(t *testing.T) { + testCases := []struct { + name string + bucketType string + context1 []string // [sub, obj, act] + context2 []string + sameKey bool + }{ + {"bucket by all", "all", []string{"alice", "data1", "read"}, []string{"bob", "data2", "write"}, true}, + {"bucket by sub", "sub", []string{"alice", "data1", "read"}, []string{"alice", "data2", "write"}, true}, + {"bucket by sub different", "sub", []string{"alice", "data1", "read"}, []string{"bob", "data1", "read"}, false}, + {"bucket by obj", "obj", []string{"alice", "data1", "read"}, []string{"bob", "data1", "write"}, true}, + {"bucket by obj different", "obj", []string{"alice", "data1", "read"}, []string{"alice", "data2", "read"}, false}, + {"bucket by act", "act", []string{"alice", "data1", "read"}, []string{"bob", "data2", "read"}, true}, + {"bucket by act different", "act", []string{"alice", "data1", "read"}, []string{"alice", "data1", "write"}, false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + eft := NewRateLimitEffector() + expr := "rate_limit(1, second, allow, " + tc.bucketType + ")" + effects := []Effect{Allow} + matches := []float64{1.0} + + // First request with context1 + eft.SetRequestContext(tc.context1[0], tc.context1[1], tc.context1[2]) + effect, _, _ := eft.MergeEffects(expr, effects, matches, 0, 1) + if effect != Allow { + t.Errorf("Expected Allow for first request, got %v", effect) + } + + // Second request with context2 + eft.SetRequestContext(tc.context2[0], tc.context2[1], tc.context2[2]) + effect, _, _ = eft.MergeEffects(expr, effects, matches, 0, 1) + + if tc.sameKey { + // Should be denied because they share the same bucket + if effect != Deny { + t.Errorf("Expected Deny (same bucket), got %v", effect) + } + } else { + // Should be allowed because they have different buckets + if effect != Allow { + t.Errorf("Expected Allow (different bucket), got %v", effect) + } + } + }) + } +} diff --git a/enforcer.go b/enforcer.go index ff8c5431f..9610a1a87 100644 --- a/enforcer.go +++ b/enforcer.go @@ -797,6 +797,34 @@ func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interfac var effect effector.Effect var explainIndex int + // If using RateLimitEffector, set request context + if rateLimitEft, ok := e.eft.(*effector.RateLimitEffector); ok { + sub, obj, act := "", "", "" + + // Extract sub from request + if subIdx, ok := rTokens[rType+"_sub"]; ok && subIdx < len(rvals) { + if subVal, ok := rvals[subIdx].(string); ok { + sub = subVal + } + } + + // Extract obj from request + if objIdx, ok := rTokens[rType+"_obj"]; ok && objIdx < len(rvals) { + if objVal, ok := rvals[objIdx].(string); ok { + obj = objVal + } + } + + // Extract act from request + if actIdx, ok := rTokens[rType+"_act"]; ok && actIdx < len(rvals) { + if actVal, ok := rvals[actIdx].(string); ok { + act = actVal + } + } + + rateLimitEft.SetRequestContext(sub, obj, act) + } + if policyLen := len(e.model["p"][pType].Policy); policyLen != 0 && strings.Contains(expString, pType+"_") { //nolint:nestif // TODO: reduce function complexity policyEffects = make([]effector.Effect, policyLen) matcherResults = make([]float64, policyLen) diff --git a/examples/rate_limit_model.conf b/examples/rate_limit_model.conf new file mode 100644 index 000000000..4fd4f57a0 --- /dev/null +++ b/examples/rate_limit_model.conf @@ -0,0 +1,11 @@ +[request_definition] +r = sub, obj, act + +[policy_definition] +p = sub, obj, act + +[policy_effect] +e = rate_limit(3, second, allow, sub) + +[matchers] +m = r.sub == p.sub && r.obj == p.obj && r.act == p.act diff --git a/examples/rate_limit_policy.csv b/examples/rate_limit_policy.csv new file mode 100644 index 000000000..9e009ab5e --- /dev/null +++ b/examples/rate_limit_policy.csv @@ -0,0 +1,4 @@ +p, alice, data1, read +p, alice, data2, write +p, bob, data1, read +p, bob, data2, write diff --git a/rate_limit_test.go b/rate_limit_test.go new file mode 100644 index 000000000..39a89d66f --- /dev/null +++ b/rate_limit_test.go @@ -0,0 +1,251 @@ +// Copyright 2026 The casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package casbin + +import ( + "testing" + "time" + + "github.com/casbin/casbin/v3/effector" + "github.com/casbin/casbin/v3/model" +) + +func TestRateLimitWithEnforcer(t *testing.T) { + e, err := NewEnforcer("examples/rate_limit_model.conf", "examples/rate_limit_policy.csv") + if err != nil { + t.Fatal(err) + } + + // Set the rate limit effector + rateLimitEft := effector.NewRateLimitEffector() + e.SetEffector(rateLimitEft) + + // Test rate limiting for alice (limit: 3 per second) + // First 3 requests should succeed + for i := 0; i < 3; i++ { + ok, err := e.Enforce("alice", "data1", "read") + if err != nil { + t.Errorf("Request %d failed: %v", i+1, err) + } + if !ok { + t.Errorf("Request %d should be allowed", i+1) + } + } + + // 4th request should be denied by rate limiter + ok, err := e.Enforce("alice", "data1", "read") + if err != nil { + t.Errorf("Request 4 failed: %v", err) + } + if ok { + t.Error("Request 4 should be denied by rate limiter") + } + + // Bob should have separate rate limit bucket + ok, err = e.Enforce("bob", "data1", "read") + if err != nil { + t.Errorf("Bob's request failed: %v", err) + } + if !ok { + t.Error("Bob's request should be allowed (separate bucket)") + } +} + +func TestRateLimitWithEnforcerWindowReset(t *testing.T) { + e, err := NewEnforcer("examples/rate_limit_model.conf", "examples/rate_limit_policy.csv") + if err != nil { + t.Fatal(err) + } + + // Set the rate limit effector + rateLimitEft := effector.NewRateLimitEffector() + e.SetEffector(rateLimitEft) + + // First 3 requests should succeed + for i := 0; i < 3; i++ { + ok, _ := e.Enforce("alice", "data1", "read") + if !ok { + t.Errorf("Request %d should be allowed", i+1) + } + } + + // 4th request should be denied + ok, _ := e.Enforce("alice", "data1", "read") + if ok { + t.Error("Request 4 should be denied by rate limiter") + } + + // Wait for window to reset + time.Sleep(1100 * time.Millisecond) + + // After window reset, request should succeed + ok, _ = e.Enforce("alice", "data1", "read") + if !ok { + t.Error("Request should be allowed after window reset") + } +} + +func TestRateLimitDifferentBucketTypes(t *testing.T) { + testCases := []struct { + name string + modelText string + requests [][]interface{} + expected []bool + }{ + { + name: "bucket by all", + modelText: ` +[request_definition] +r = sub, obj, act + +[policy_definition] +p = sub, obj, act + +[policy_effect] +e = rate_limit(2, second, allow, all) + +[matchers] +m = r.sub == p.sub && r.obj == p.obj && r.act == p.act +`, + requests: [][]interface{}{ + {"alice", "data1", "read"}, // 1st - allowed + {"bob", "data1", "read"}, // 2nd - allowed (shares bucket with alice) + {"alice", "data2", "write"}, // 3rd - denied (same bucket) + }, + expected: []bool{true, true, false}, + }, + { + name: "bucket by obj", + modelText: ` +[request_definition] +r = sub, obj, act + +[policy_definition] +p = sub, obj, act + +[policy_effect] +e = rate_limit(2, second, allow, obj) + +[matchers] +m = r.sub == p.sub && r.obj == p.obj && r.act == p.act +`, + requests: [][]interface{}{ + {"alice", "data1", "read"}, // 1st - allowed + {"bob", "data1", "read"}, // 2nd - allowed (same obj bucket) + {"alice", "data1", "write"}, // 3rd - denied (same obj bucket) + {"alice", "data2", "write"}, // 4th - allowed (different obj bucket) + }, + expected: []bool{true, true, false, true}, + }, + { + name: "bucket by act", + modelText: ` +[request_definition] +r = sub, obj, act + +[policy_definition] +p = sub, obj, act + +[policy_effect] +e = rate_limit(2, second, allow, act) + +[matchers] +m = r.sub == p.sub && r.obj == p.obj && r.act == p.act +`, + requests: [][]interface{}{ + {"alice", "data1", "read"}, // 1st - allowed + {"bob", "data2", "read"}, // 2nd - allowed (same act bucket) + {"alice", "data1", "read"}, // 3rd - denied (same act bucket) + {"alice", "data1", "write"}, // 4th - allowed (different act bucket) + }, + expected: []bool{true, true, false, true}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + m, _ := model.NewModelFromString(tc.modelText) + e, _ := NewEnforcer(m) + + // Add policies + e.AddPolicy("alice", "data1", "read") + e.AddPolicy("alice", "data1", "write") + e.AddPolicy("alice", "data2", "write") + e.AddPolicy("bob", "data1", "read") + e.AddPolicy("bob", "data2", "read") + e.AddPolicy("bob", "data2", "write") + + // Set the rate limit effector + rateLimitEft := effector.NewRateLimitEffector() + e.SetEffector(rateLimitEft) + + // Test requests + for i, req := range tc.requests { + ok, err := e.Enforce(req...) + if err != nil { + t.Errorf("Request %d failed: %v", i+1, err) + } + if ok != tc.expected[i] { + t.Errorf("Request %d (sub=%v, obj=%v, act=%v): expected %v, got %v", + i+1, req[0], req[1], req[2], tc.expected[i], ok) + } + } + }) + } +} + +func TestRateLimitCountTypeAll(t *testing.T) { + modelText := ` +[request_definition] +r = sub, obj, act + +[policy_definition] +p = sub, obj, act + +[policy_effect] +e = rate_limit(2, second, all, sub) + +[matchers] +m = r.sub == p.sub && r.obj == p.obj && r.act == p.act +` + m, _ := model.NewModelFromString(modelText) + e, _ := NewEnforcer(m) + + // Add policy only for alice/data1/read + e.AddPolicy("alice", "data1", "read") + + // Set the rate limit effector + rateLimitEft := effector.NewRateLimitEffector() + e.SetEffector(rateLimitEft) + + // First request - matches policy (allowed) + ok, _ := e.Enforce("alice", "data1", "read") + if !ok { + t.Error("First request should be allowed") + } + + // Second request - doesn't match policy (denied by policy, but counts) + ok, _ = e.Enforce("alice", "data2", "write") + if ok { + t.Error("Second request should be denied by policy") + } + + // Third request - should be denied by rate limiter even if policy matches + // (because count_type is "all" and we already counted 2 requests) + ok, _ = e.Enforce("alice", "data1", "read") + if ok { + t.Error("Third request should be denied by rate limiter") + } +} From bba4ac4521cbf1c15b3dbf966c23f12f911f74ea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 8 Jan 2026 18:05:03 +0000 Subject: [PATCH 3/7] Add rate limiting example and documentation Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- examples/rate_limit_example/README.md | 166 ++++++++++++++++++++++++++ examples/rate_limit_example/main.go | 73 +++++++++++ 2 files changed, 239 insertions(+) create mode 100644 examples/rate_limit_example/README.md create mode 100644 examples/rate_limit_example/main.go diff --git a/examples/rate_limit_example/README.md b/examples/rate_limit_example/README.md new file mode 100644 index 000000000..79a240e6f --- /dev/null +++ b/examples/rate_limit_example/README.md @@ -0,0 +1,166 @@ +# Rate Limiting with Casbin + +This example demonstrates how to use Casbin's rate limiting feature through the `RateLimitEffector`. + +## Overview + +Rate limiting in Casbin allows you to control the rate of requests based on various criteria such as subject, object, or action. This is useful for: + +- Preventing abuse and brute-force attacks +- Ensuring fair resource allocation among users +- Protecting backend services from overload + +## Configuration + +### Model Configuration + +To enable rate limiting, use the `rate_limit()` function in your policy effect definition: + +```ini +[policy_effect] +e = rate_limit(max, unit, count_type, bucket) +``` + +**Parameters:** + +- `max`: Maximum number of requests allowed within the time window (integer) +- `unit`: Time window unit - can be `second`, `minute`, `hour`, or `day` +- `count_type`: What to count: + - `allow`: Only count allowed requests (useful for API quotas) + - `all`: Count all requests including denied ones (useful for preventing brute-force attacks) +- `bucket`: How to group requests: + - `sub`: Separate bucket per subject (user-based rate limiting) + - `obj`: Separate bucket per object (resource-based rate limiting) + - `act`: Separate bucket per action (operation-based rate limiting) + - `all`: Single bucket for all requests (global rate limiting) + +### Example Models + +**User-based rate limiting (3 requests per second per user):** +```ini +[request_definition] +r = sub, obj, act + +[policy_definition] +p = sub, obj, act + +[policy_effect] +e = rate_limit(3, second, allow, sub) + +[matchers] +m = r.sub == p.sub && r.obj == p.obj && r.act == p.act +``` + +**Global rate limiting (100 requests per minute for all users):** +```ini +[policy_effect] +e = rate_limit(100, minute, allow, all) +``` + +**Resource-based rate limiting (10 requests per hour per resource):** +```ini +[policy_effect] +e = rate_limit(10, hour, allow, obj) +``` + +**Brute-force protection (count all attempts, not just allowed ones):** +```ini +[policy_effect] +e = rate_limit(5, minute, all, sub) +``` + +## Usage + +To use rate limiting in your code: + +```go +package main + +import ( + "github.com/casbin/casbin/v3" + "github.com/casbin/casbin/v3/effector" +) + +func main() { + // Create enforcer with rate limit model + e, err := casbin.NewEnforcer("model.conf", "policy.csv") + if err != nil { + panic(err) + } + + // Set the rate limit effector (required!) + rateLimitEft := effector.NewRateLimitEffector() + e.SetEffector(rateLimitEft) + + // Now enforce with rate limiting + ok, err := e.Enforce("alice", "data1", "read") + if err != nil { + // Handle error + } + if !ok { + // Request denied (either by policy or rate limit) + } +} +``` + +## Running the Example + +```bash +cd examples/rate_limit_example +go run main.go +``` + +## How It Works + +1. The `RateLimitEffector` maintains internal state for each bucket (counter and window expiration time) +2. When a request is enforced, the effector: + - First checks if the request matches any policy rules + - If it should be counted (based on `count_type`), updates the appropriate bucket counter + - If the counter exceeds the limit, denies the request + - When the time window expires, the counter is reset + +3. Buckets are isolated based on the `bucket` parameter: + - `sub`: Each subject (user) has its own bucket + - `obj`: Each object (resource) has its own bucket + - `act`: Each action has its own bucket + - `all`: All requests share a single bucket + +## Important Notes + +- **You must call `SetEffector()` to enable rate limiting.** Without it, the default effector will be used. +- Rate limiting is stateful and maintained in memory. If your application restarts, counters are reset. +- The time windows are sliding windows - they start from the first request in the window. +- Bucket keys are automatically generated from the request context based on the bucket type. + +## Advanced Use Cases + +### Combining with RBAC + +You can use rate limiting together with RBAC models: + +```ini +[request_definition] +r = sub, obj, act + +[policy_definition] +p = sub, obj, act + +[role_definition] +g = _, _ + +[policy_effect] +e = rate_limit(10, minute, allow, sub) + +[matchers] +m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act +``` + +### Different Limits for Different Users + +To implement different rate limits for different users or roles, you would need to use multiple enforcers or implement a custom effector that reads limit values from policies. + +## See Also + +- [Main Casbin Documentation](https://casbin.org/docs/overview) +- [Policy Effects](https://casbin.org/docs/syntax-for-models#policy-effect) +- [Effector Interface](../../effector/effector.go) diff --git a/examples/rate_limit_example/main.go b/examples/rate_limit_example/main.go new file mode 100644 index 000000000..d43ba891e --- /dev/null +++ b/examples/rate_limit_example/main.go @@ -0,0 +1,73 @@ +// Copyright 2026 The casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( +"fmt" + +"github.com/casbin/casbin/v3" +"github.com/casbin/casbin/v3/effector" +) + +func main() { +// Create an enforcer with a rate limit model +e, err := casbin.NewEnforcer("../rate_limit_model.conf", "../rate_limit_policy.csv") +if err != nil { +panic(err) +} + +// Set the rate limit effector +// This is required to enable rate limiting functionality +rateLimitEft := effector.NewRateLimitEffector() +e.SetEffector(rateLimitEft) + +fmt.Println("Rate Limiting Example") +fmt.Println("======================") +fmt.Println("Policy: rate_limit(3, second, allow, sub)") +fmt.Println("This means: Allow at most 3 requests per second, per subject") +fmt.Println() + +// Alice tries to access data1 with read permission +// The rate limit is 3 per second, so the first 3 should succeed +for i := 1; i <= 5; i++ { +ok, err := e.Enforce("alice", "data1", "read") +if err != nil { +fmt.Printf("Request %d error: %v\n", i, err) +continue +} +if ok { +fmt.Printf("Request %d: ✓ Allowed\n", i) +} else { +fmt.Printf("Request %d: ✗ Denied (rate limit exceeded)\n", i) +} +} + +fmt.Println() +fmt.Println("Bob has a separate rate limit bucket:") + +// Bob should have a separate rate limit bucket +for i := 1; i <= 3; i++ { +ok, err := e.Enforce("bob", "data1", "read") +if err != nil { +fmt.Printf("Bob's request %d error: %v\n", i, err) +continue +} +if ok { +fmt.Printf("Bob's request %d: ✓ Allowed\n", i) +} else { +fmt.Printf("Bob's request %d: ✗ Denied\n", i) +} +} +} From 84b0ca7f559ba18e1351aa115925fe40ff10169a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 8 Jan 2026 18:07:44 +0000 Subject: [PATCH 4/7] Fix race condition and code formatting issues from code review Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- effector/rate_limit_effector.go | 11 ++++++----- examples/rate_limit_example/main.go | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/effector/rate_limit_effector.go b/effector/rate_limit_effector.go index 0d730d1a6..da11ebc72 100644 --- a/effector/rate_limit_effector.go +++ b/effector/rate_limit_effector.go @@ -141,9 +141,6 @@ func (e *RateLimitEffector) MergeEffects(expr string, effects []Effect, matches shouldCount = true } - // Generate bucket key based on bucket type and request context - bucketKey := e.generateBucketKey(bucketType) - // Check and update rate limit if shouldCount { now := time.Now() @@ -152,6 +149,9 @@ func (e *RateLimitEffector) MergeEffects(expr string, effects []Effect, matches e.mu.Lock() defer e.mu.Unlock() + // Generate bucket key inside lock to avoid race condition + bucketKey := e.generateBucketKeyLocked(bucketType) + bucket, exists := e.buckets[bucketKey] if !exists || now.After(bucket.windowEnd) { // Create new window @@ -182,8 +182,9 @@ func (e *RateLimitEffector) SetRequestContext(sub, obj, act string) { e.requestContext["act"] = act } -// generateBucketKey generates a bucket key based on the bucket type and request context -func (e *RateLimitEffector) generateBucketKey(bucketType string) string { +// generateBucketKeyLocked generates a bucket key based on the bucket type and request context +// Must be called with e.mu held +func (e *RateLimitEffector) generateBucketKeyLocked(bucketType string) string { switch bucketType { case "all": return "bucket:all" diff --git a/examples/rate_limit_example/main.go b/examples/rate_limit_example/main.go index d43ba891e..37f69f31c 100644 --- a/examples/rate_limit_example/main.go +++ b/examples/rate_limit_example/main.go @@ -15,10 +15,10 @@ package main import ( -"fmt" + "fmt" -"github.com/casbin/casbin/v3" -"github.com/casbin/casbin/v3/effector" + "github.com/casbin/casbin/v3" + "github.com/casbin/casbin/v3/effector" ) func main() { From 7296044ed71897cf3febe5094aec01cee16cb8cc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 8 Jan 2026 18:10:24 +0000 Subject: [PATCH 5/7] Address final code review feedback: improve comments and error handling Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- effector/rate_limit_effector.go | 2 +- examples/rate_limit_example/main.go | 86 ++++++++++++++--------------- rate_limit_test.go | 20 +++++-- 3 files changed, 60 insertions(+), 48 deletions(-) diff --git a/effector/rate_limit_effector.go b/effector/rate_limit_effector.go index da11ebc72..0f9e1a778 100644 --- a/effector/rate_limit_effector.go +++ b/effector/rate_limit_effector.go @@ -183,7 +183,7 @@ func (e *RateLimitEffector) SetRequestContext(sub, obj, act string) { } // generateBucketKeyLocked generates a bucket key based on the bucket type and request context -// Must be called with e.mu held +// IMPORTANT: Must be called with e.mu held to avoid race conditions func (e *RateLimitEffector) generateBucketKeyLocked(bucketType string) string { switch bucketType { case "all": diff --git a/examples/rate_limit_example/main.go b/examples/rate_limit_example/main.go index 37f69f31c..a9bf89707 100644 --- a/examples/rate_limit_example/main.go +++ b/examples/rate_limit_example/main.go @@ -22,52 +22,52 @@ import ( ) func main() { -// Create an enforcer with a rate limit model -e, err := casbin.NewEnforcer("../rate_limit_model.conf", "../rate_limit_policy.csv") -if err != nil { -panic(err) -} + // Create an enforcer with a rate limit model + e, err := casbin.NewEnforcer("../rate_limit_model.conf", "../rate_limit_policy.csv") + if err != nil { + panic(err) + } -// Set the rate limit effector -// This is required to enable rate limiting functionality -rateLimitEft := effector.NewRateLimitEffector() -e.SetEffector(rateLimitEft) + // Set the rate limit effector + // This is required to enable rate limiting functionality + rateLimitEft := effector.NewRateLimitEffector() + e.SetEffector(rateLimitEft) -fmt.Println("Rate Limiting Example") -fmt.Println("======================") -fmt.Println("Policy: rate_limit(3, second, allow, sub)") -fmt.Println("This means: Allow at most 3 requests per second, per subject") -fmt.Println() + fmt.Println("Rate Limiting Example") + fmt.Println("======================") + fmt.Println("Policy: rate_limit(3, second, allow, sub)") + fmt.Println("This means: Allow at most 3 requests per second, per subject") + fmt.Println() -// Alice tries to access data1 with read permission -// The rate limit is 3 per second, so the first 3 should succeed -for i := 1; i <= 5; i++ { -ok, err := e.Enforce("alice", "data1", "read") -if err != nil { -fmt.Printf("Request %d error: %v\n", i, err) -continue -} -if ok { -fmt.Printf("Request %d: ✓ Allowed\n", i) -} else { -fmt.Printf("Request %d: ✗ Denied (rate limit exceeded)\n", i) -} -} + // Alice tries to access data1 with read permission + // The rate limit is 3 per second, so the first 3 should succeed + for i := 1; i <= 5; i++ { + ok, err := e.Enforce("alice", "data1", "read") + if err != nil { + fmt.Printf("Request %d error: %v\n", i, err) + continue + } + if ok { + fmt.Printf("Request %d: ✓ Allowed\n", i) + } else { + fmt.Printf("Request %d: ✗ Denied (rate limit exceeded)\n", i) + } + } -fmt.Println() -fmt.Println("Bob has a separate rate limit bucket:") + fmt.Println() + fmt.Println("Bob has a separate rate limit bucket:") -// Bob should have a separate rate limit bucket -for i := 1; i <= 3; i++ { -ok, err := e.Enforce("bob", "data1", "read") -if err != nil { -fmt.Printf("Bob's request %d error: %v\n", i, err) -continue -} -if ok { -fmt.Printf("Bob's request %d: ✓ Allowed\n", i) -} else { -fmt.Printf("Bob's request %d: ✗ Denied\n", i) -} -} + // Bob should have a separate rate limit bucket + for i := 1; i <= 3; i++ { + ok, err := e.Enforce("bob", "data1", "read") + if err != nil { + fmt.Printf("Bob's request %d error: %v\n", i, err) + continue + } + if ok { + fmt.Printf("Bob's request %d: ✓ Allowed\n", i) + } else { + fmt.Printf("Bob's request %d: ✗ Denied\n", i) + } + } } diff --git a/rate_limit_test.go b/rate_limit_test.go index 39a89d66f..70a3be19f 100644 --- a/rate_limit_test.go +++ b/rate_limit_test.go @@ -176,8 +176,14 @@ m = r.sub == p.sub && r.obj == p.obj && r.act == p.act for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - m, _ := model.NewModelFromString(tc.modelText) - e, _ := NewEnforcer(m) + m, err := model.NewModelFromString(tc.modelText) + if err != nil { + t.Fatalf("Failed to create model: %v", err) + } + e, err := NewEnforcer(m) + if err != nil { + t.Fatalf("Failed to create enforcer: %v", err) + } // Add policies e.AddPolicy("alice", "data1", "read") @@ -220,8 +226,14 @@ e = rate_limit(2, second, all, sub) [matchers] m = r.sub == p.sub && r.obj == p.obj && r.act == p.act ` - m, _ := model.NewModelFromString(modelText) - e, _ := NewEnforcer(m) + m, err := model.NewModelFromString(modelText) + if err != nil { + t.Fatalf("Failed to create model: %v", err) + } + e, err := NewEnforcer(m) + if err != nil { + t.Fatalf("Failed to create enforcer: %v", err) + } // Add policy only for alice/data1/read e.AddPolicy("alice", "data1", "read") From 67c4d1460a9222efbac0e301a2bd9d6bccefe4b4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 9 Jan 2026 11:44:18 +0000 Subject: [PATCH 6/7] Fix golangci-lint godot errors: add periods to comment endings Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- effector/rate_limit_effector.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/effector/rate_limit_effector.go b/effector/rate_limit_effector.go index 0f9e1a778..45be5e460 100644 --- a/effector/rate_limit_effector.go +++ b/effector/rate_limit_effector.go @@ -24,20 +24,20 @@ import ( "time" ) -// RateLimitBucket holds the state for a rate limit bucket +// RateLimitBucket holds the state for a rate limit bucket. type RateLimitBucket struct { count int windowEnd time.Time } -// RateLimitEffector is an effector that implements rate limiting +// RateLimitEffector is an effector that implements rate limiting. type RateLimitEffector struct { mu sync.RWMutex buckets map[string]*RateLimitBucket requestContext map[string]string // stores current request context (sub, obj, act) } -// NewRateLimitEffector creates a new RateLimitEffector +// NewRateLimitEffector creates a new RateLimitEffector. func NewRateLimitEffector() *RateLimitEffector { return &RateLimitEffector{ buckets: make(map[string]*RateLimitBucket), @@ -47,8 +47,8 @@ func NewRateLimitEffector() *RateLimitEffector { var rateLimitRegex = regexp.MustCompile(`rate_limit\((\d+),\s*(\w+),\s*(\w+),\s*(\w+)\)`) -// parseRateLimitExpr parses a rate_limit expression -// Format: rate_limit(max, unit, count_type, bucket) +// parseRateLimitExpr parses a rate_limit expression. +// Format: rate_limit(max, unit, count_type, bucket). func parseRateLimitExpr(expr string) (max int, unit string, countType string, bucket string, err error) { matches := rateLimitRegex.FindStringSubmatch(expr) if matches == nil || len(matches) != 5 { @@ -84,7 +84,7 @@ func parseRateLimitExpr(expr string) (max int, unit string, countType string, bu return max, unit, countType, bucket, nil } -// getWindowDuration returns the duration for a time unit +// getWindowDuration returns the duration for a time unit. func getWindowDuration(unit string) time.Duration { switch unit { case "second": @@ -100,7 +100,7 @@ func getWindowDuration(unit string) time.Duration { } } -// MergeEffects implements the Effector interface with rate limiting +// MergeEffects implements the Effector interface with rate limiting. func (e *RateLimitEffector) MergeEffects(expr string, effects []Effect, matches []float64, policyIndex int, policyLength int) (Effect, int, error) { // Check if this is a rate_limit expression if !strings.Contains(expr, "rate_limit") { @@ -172,8 +172,8 @@ func (e *RateLimitEffector) MergeEffects(expr string, effects []Effect, matches return baseEffect, explainIndex, nil } -// SetRequestContext sets the request context for bucket key generation -// This should be called before MergeEffects to provide request context +// SetRequestContext sets the request context for bucket key generation. +// This should be called before MergeEffects to provide request context. func (e *RateLimitEffector) SetRequestContext(sub, obj, act string) { e.mu.Lock() defer e.mu.Unlock() @@ -182,8 +182,8 @@ func (e *RateLimitEffector) SetRequestContext(sub, obj, act string) { e.requestContext["act"] = act } -// generateBucketKeyLocked generates a bucket key based on the bucket type and request context -// IMPORTANT: Must be called with e.mu held to avoid race conditions +// generateBucketKeyLocked generates a bucket key based on the bucket type and request context. +// IMPORTANT: Must be called with e.mu held to avoid race conditions. func (e *RateLimitEffector) generateBucketKeyLocked(bucketType string) string { switch bucketType { case "all": @@ -208,7 +208,7 @@ func (e *RateLimitEffector) generateBucketKeyLocked(bucketType string) string { } } -// GetBucketState returns the current state of a bucket (for testing) +// GetBucketState returns the current state of a bucket (for testing). func (e *RateLimitEffector) GetBucketState(key string) (count int, windowEnd time.Time, exists bool) { e.mu.RLock() defer e.mu.RUnlock() @@ -220,7 +220,7 @@ func (e *RateLimitEffector) GetBucketState(key string) (count int, windowEnd tim return bucket.count, bucket.windowEnd, true } -// ResetBuckets clears all buckets (for testing) +// ResetBuckets clears all buckets (for testing). func (e *RateLimitEffector) ResetBuckets() { e.mu.Lock() defer e.mu.Unlock() From 922f56e98e05a345e223cbbd2b9b80e20bc2b586 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 9 Jan 2026 12:00:07 +0000 Subject: [PATCH 7/7] Fix golangci-lint errors: remove unused nolint, fix type inference, shadow var, reduce nesting Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- effector/effector.go | 2 +- effector/rate_limit_effector.go | 4 ++-- enforcer.go | 36 ++++++++++++--------------------- rate_limit_test.go | 16 +++++++-------- 4 files changed, 24 insertions(+), 34 deletions(-) diff --git a/effector/effector.go b/effector/effector.go index 49b84c3e1..665848b5c 100644 --- a/effector/effector.go +++ b/effector/effector.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package effector //nolint:cyclop // TODO +package effector // Effect is the result for a policy rule. type Effect int diff --git a/effector/rate_limit_effector.go b/effector/rate_limit_effector.go index 45be5e460..ba166de4f 100644 --- a/effector/rate_limit_effector.go +++ b/effector/rate_limit_effector.go @@ -113,8 +113,8 @@ func (e *RateLimitEffector) MergeEffects(expr string, effects []Effect, matches } // For rate limiting, we need to check all matches first to determine the base result - var baseEffect Effect = Indeterminate - var explainIndex int = -1 + var baseEffect = Indeterminate + var explainIndex = -1 // Find the first matching policy for i := 0; i < policyLength; i++ { diff --git a/enforcer.go b/enforcer.go index 9610a1a87..29962f329 100644 --- a/enforcer.go +++ b/enforcer.go @@ -677,6 +677,16 @@ func (e *Enforcer) invalidateMatcherMap() { e.matcherMap = sync.Map{} } +// extractRequestParam extracts a string parameter from request values based on token name. +func extractRequestParam(rvals []interface{}, rTokens map[string]int, tokenName string) string { + if idx, ok := rTokens[tokenName]; ok && idx < len(rvals) { + if val, ok := rvals[idx].(string); ok { + return val + } + } + return "" +} + // enforce use a custom matcher to decides whether a "subject" can access a "object" with the operation "action", input parameters are usually: (matcher, sub, obj, act), use model matcher by default when matcher is "". func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interface{}) (ok bool, err error) { //nolint:funlen,cyclop,gocyclo // TODO: reduce function complexity logEntry := e.onLogBeforeEventInEnforce(rvals) @@ -799,29 +809,9 @@ func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interfac // If using RateLimitEffector, set request context if rateLimitEft, ok := e.eft.(*effector.RateLimitEffector); ok { - sub, obj, act := "", "", "" - - // Extract sub from request - if subIdx, ok := rTokens[rType+"_sub"]; ok && subIdx < len(rvals) { - if subVal, ok := rvals[subIdx].(string); ok { - sub = subVal - } - } - - // Extract obj from request - if objIdx, ok := rTokens[rType+"_obj"]; ok && objIdx < len(rvals) { - if objVal, ok := rvals[objIdx].(string); ok { - obj = objVal - } - } - - // Extract act from request - if actIdx, ok := rTokens[rType+"_act"]; ok && actIdx < len(rvals) { - if actVal, ok := rvals[actIdx].(string); ok { - act = actVal - } - } - + sub := extractRequestParam(rvals, rTokens, rType+"_sub") + obj := extractRequestParam(rvals, rTokens, rType+"_obj") + act := extractRequestParam(rvals, rTokens, rType+"_act") rateLimitEft.SetRequestContext(sub, obj, act) } diff --git a/rate_limit_test.go b/rate_limit_test.go index 70a3be19f..11c8a24ae 100644 --- a/rate_limit_test.go +++ b/rate_limit_test.go @@ -35,9 +35,9 @@ func TestRateLimitWithEnforcer(t *testing.T) { // Test rate limiting for alice (limit: 3 per second) // First 3 requests should succeed for i := 0; i < 3; i++ { - ok, err := e.Enforce("alice", "data1", "read") - if err != nil { - t.Errorf("Request %d failed: %v", i+1, err) + ok, err2 := e.Enforce("alice", "data1", "read") + if err2 != nil { + t.Errorf("Request %d failed: %v", i+1, err2) } if !ok { t.Errorf("Request %d should be allowed", i+1) @@ -99,10 +99,10 @@ func TestRateLimitWithEnforcerWindowReset(t *testing.T) { func TestRateLimitDifferentBucketTypes(t *testing.T) { testCases := []struct { - name string - modelText string - requests [][]interface{} - expected []bool + name string + modelText string + requests [][]interface{} + expected []bool }{ { name: "bucket by all", @@ -184,7 +184,7 @@ m = r.sub == p.sub && r.obj == p.obj && r.act == p.act if err != nil { t.Fatalf("Failed to create enforcer: %v", err) } - + // Add policies e.AddPolicy("alice", "data1", "read") e.AddPolicy("alice", "data1", "write")