From 70df03ffbb203a48a1d17deaa8280f9cc5713487 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Mon, 18 Nov 2024 14:46:06 +0200 Subject: [PATCH] fixed rate limiter rules matching to acount for the Audience field --- CHANGELOG.md | 2 + apis/middlewares_rate_limit.go | 22 ++++++++-- apis/middlewares_rate_limit_test.go | 18 ++++---- core/settings_model.go | 12 ++++-- core/settings_model_test.go | 42 ++++++++++++------- .../components/settings/BatchAccordion.svelte | 2 +- .../settings/RateLimitAccordion.svelte | 18 +++++++- 7 files changed, 82 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aba99b7b..d6ba9b6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ > [!CAUTION] > **This is a prerelease intended for test and experimental purposes only!** +- Fixed rate limiter rules matching to acount for the `Audience` field. + - Minor UI fixes (fixed duplicate record control, removed duplicated id field in the record preview, hide Impersonate button for non-auth records, etc.). diff --git a/apis/middlewares_rate_limit.go b/apis/middlewares_rate_limit.go index a6e7dd11..f794eb86 100644 --- a/apis/middlewares_rate_limit.go +++ b/apis/middlewares_rate_limit.go @@ -32,9 +32,12 @@ func rateLimit() *hook.Handler[*core.RequestEvent] { return e.Next() } - rule, ok := e.App.Settings().RateLimits.FindRateLimitRule(defaultRateLimitLabels(e)) + rule, ok := e.App.Settings().RateLimits.FindRateLimitRule( + defaultRateLimitLabels(e), + defaultRateLimitAudience(e)..., + ) if ok { - err := checkRateLimit(e, e.Request.Pattern, rule) + err := checkRateLimit(e, rule.Label+rule.Audience, rule) if err != nil { return err } @@ -94,9 +97,9 @@ func checkCollectionRateLimit(e *core.RequestEvent, collection *core.Collection, } labels = append(labels, defaultRateLimitLabels(e)...) - rule, ok := e.App.Settings().RateLimits.FindRateLimitRule(labels) + rule, ok := e.App.Settings().RateLimits.FindRateLimitRule(labels, defaultRateLimitAudience(e)...) if ok { - return checkRateLimit(e, rtId, rule) + return checkRateLimit(e, rtId+rule.Audience, rule) } return nil @@ -174,6 +177,17 @@ func skipRateLimit(e *core.RequestEvent) bool { return !e.App.Settings().RateLimits.Enabled || e.HasSuperuserAuth() } +var defaultAuthAudience = []string{core.RateLimitRuleAudienceAll, core.RateLimitRuleAudienceAuth} +var defaultGuestAudience = []string{core.RateLimitRuleAudienceAll, core.RateLimitRuleAudienceGuest} + +func defaultRateLimitAudience(e *core.RequestEvent) []string { + if e.Auth != nil { + return defaultAuthAudience + } + + return defaultGuestAudience +} + func defaultRateLimitLabels(e *core.RequestEvent) []string { return []string{e.Request.Method + " " + e.Request.URL.Path, e.Request.URL.Path} } diff --git a/apis/middlewares_rate_limit_test.go b/apis/middlewares_rate_limit_test.go index 093e6c53..bbcb9850 100644 --- a/apis/middlewares_rate_limit_test.go +++ b/apis/middlewares_rate_limit_test.go @@ -101,27 +101,27 @@ func TestDefaultRateLimitMiddleware(t *testing.T) { {"/rate/b", 0, false, 200}, {"/rate/b", 0, false, 429}, - // "auth" with guest (should be ignored) - {"/rate/auth", 0, false, 200}, - {"/rate/auth", 0, false, 200}, + // "auth" with guest (should fallback to the /rate/ rule) {"/rate/auth", 0, false, 200}, {"/rate/auth", 0, false, 200}, + {"/rate/auth", 0, false, 429}, + {"/rate/auth", 0, false, 429}, - // "auth" rule with regular user + // "auth" rule with regular user (should match the /rate/auth rule) {"/rate/auth", 0, true, 200}, {"/rate/auth", 0, true, 429}, {"/rate/auth", 0, true, 429}, - // "guest" with guest + // "guest" with guest (should match the /rate/guest rule) {"/rate/guest", 0, false, 200}, {"/rate/guest", 0, false, 429}, {"/rate/guest", 0, false, 429}, - // "guest" rule with regular user (should be ignored) - {"/rate/guest", 0, true, 200}, - {"/rate/guest", 0, true, 200}, - {"/rate/guest", 0, true, 200}, + // "guest" rule with regular user (should fallback to the /rate/ rule) + {"/rate/guest", 1, true, 200}, {"/rate/guest", 0, true, 200}, + {"/rate/guest", 0, true, 429}, + {"/rate/guest", 0, true, 429}, } for _, s := range scenarios { diff --git a/core/settings_model.go b/core/settings_model.go index 81c145bd..fd05b96e 100644 --- a/core/settings_model.go +++ b/core/settings_model.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "regexp" + "slices" "strconv" "strings" "sync" @@ -560,13 +561,17 @@ type RateLimitsConfig struct { } // FindRateLimitRule returns the first matching rule based on the provided labels. -func (c *RateLimitsConfig) FindRateLimitRule(searchLabels []string) (RateLimitRule, bool) { +// +// Optionally you can further specify a list of valid RateLimitRule.Audience values to further filter the matching rule +// (aka. the rule Audience will have to exist in one of the specified options). +func (c *RateLimitsConfig) FindRateLimitRule(searchLabels []string, optOnlyAudience ...string) (RateLimitRule, bool) { var prefixRules []int for i, label := range searchLabels { // check for direct match for j := range c.Rules { - if label == c.Rules[j].Label { + if label == c.Rules[j].Label && + (len(optOnlyAudience) == 0 || slices.Contains(optOnlyAudience, c.Rules[j].Audience)) { return c.Rules[j], true } @@ -578,7 +583,8 @@ func (c *RateLimitsConfig) FindRateLimitRule(searchLabels []string) (RateLimitRu // check for prefix match if len(prefixRules) > 0 { for j := range prefixRules { - if strings.HasPrefix(label+"/", c.Rules[prefixRules[j]].Label) { + if strings.HasPrefix(label+"/", c.Rules[prefixRules[j]].Label) && + (len(optOnlyAudience) == 0 || slices.Contains(optOnlyAudience, c.Rules[prefixRules[j]].Audience)) { return c.Rules[prefixRules[j]], true } } diff --git a/core/settings_model_test.go b/core/settings_model_test.go index 51800d8a..7706085e 100644 --- a/core/settings_model_test.go +++ b/core/settings_model_test.go @@ -634,33 +634,43 @@ func TestRateLimitsFindRateLimitRule(t *testing.T) { limits := core.RateLimitsConfig{ Rules: []core.RateLimitRule{ {Label: "abc"}, - {Label: "POST /test/a/"}, - {Label: "/test/a/"}, + {Label: "def", Audience: core.RateLimitRuleAudienceGuest}, + {Label: "/test/a", Audience: core.RateLimitRuleAudienceGuest}, {Label: "POST /test/a"}, - {Label: "/test/a"}, + {Label: "/test/a/", Audience: core.RateLimitRuleAudienceAuth}, + {Label: "POST /test/a/"}, }, } scenarios := []struct { labels []string + audience []string expected string }{ - {[]string{}, ""}, - {[]string{"missing"}, ""}, - {[]string{"abc"}, "abc"}, - {[]string{"/test"}, ""}, - {[]string{"/test/a"}, "/test/a"}, - {[]string{"GET /test/a"}, ""}, - {[]string{"POST /test/a"}, "POST /test/a"}, - {[]string{"/test/a/b/c"}, "/test/a/"}, - {[]string{"GET /test/a/b/c"}, ""}, - {[]string{"POST /test/a/b/c"}, "POST /test/a/"}, - {[]string{"/test/a", "abc"}, "/test/a"}, // priority checks + {[]string{}, []string{}, ""}, + {[]string{"missing"}, []string{}, ""}, + {[]string{"abc"}, []string{}, "abc"}, + {[]string{"abc"}, []string{core.RateLimitRuleAudienceGuest}, ""}, + {[]string{"abc"}, []string{core.RateLimitRuleAudienceAuth}, ""}, + {[]string{"def"}, []string{core.RateLimitRuleAudienceGuest}, "def"}, + {[]string{"def"}, []string{core.RateLimitRuleAudienceAuth}, ""}, + {[]string{"/test"}, []string{}, ""}, + {[]string{"/test/a"}, []string{}, "/test/a"}, + {[]string{"/test/a"}, []string{core.RateLimitRuleAudienceAuth}, "/test/a/"}, + {[]string{"/test/a"}, []string{core.RateLimitRuleAudienceGuest}, "/test/a"}, + {[]string{"GET /test/a"}, []string{}, ""}, + {[]string{"POST /test/a"}, []string{}, "POST /test/a"}, + {[]string{"/test/a/b/c"}, []string{}, "/test/a/"}, + {[]string{"/test/a/b/c"}, []string{core.RateLimitRuleAudienceAuth}, "/test/a/"}, + {[]string{"/test/a/b/c"}, []string{core.RateLimitRuleAudienceGuest}, ""}, + {[]string{"GET /test/a/b/c"}, []string{}, ""}, + {[]string{"POST /test/a/b/c"}, []string{}, "POST /test/a/"}, + {[]string{"/test/a", "abc"}, []string{}, "/test/a"}, // priority checks } for _, s := range scenarios { - t.Run(strings.Join(s.labels, ""), func(t *testing.T) { - rule, ok := limits.FindRateLimitRule(s.labels) + t.Run(strings.Join(s.labels, "_")+":"+strings.Join(s.audience, "_"), func(t *testing.T) { + rule, ok := limits.FindRateLimitRule(s.labels, s.audience...) hasLabel := rule.Label != "" if hasLabel != ok { diff --git a/ui/src/components/settings/BatchAccordion.svelte b/ui/src/components/settings/BatchAccordion.svelte index 21c0e9b3..62908779 100644 --- a/ui/src/components/settings/BatchAccordion.svelte +++ b/ui/src/components/settings/BatchAccordion.svelte @@ -39,7 +39,7 @@ - +
diff --git a/ui/src/components/settings/RateLimitAccordion.svelte b/ui/src/components/settings/RateLimitAccordion.svelte index e77fef94..1ed9353a 100644 --- a/ui/src/components/settings/RateLimitAccordion.svelte +++ b/ui/src/components/settings/RateLimitAccordion.svelte @@ -150,7 +150,7 @@ - + {#if !CommonHelper.isEmpty(formSettings.rateLimits.rules)} @@ -263,6 +263,22 @@

Rate limit label format

+

The rate limit rules are resolved in the following order (stops on the first match):

+
    +
  1. exact tag (e.g. users:create)
    2. +
  2. wildcard tag (e.g. *:create)
    3. +
  3. METHOD + exact path (e.g. POST /a/b)
    4. +
  4. METHOD + prefix path (e.g. POST /a/b/)
    5. +
  5. exact path (e.g. /a/b)
    6. +
  6. prefix path (e.g. /a/b/)
    7. +
+

+ In case of multiple rules with the same label but different target user audience (e.g. "guest" vs + "auth"), only the matching audience rule is taken in consideration. +

+ +
+

The rate limit label could be in one of the following formats: