From 2c7eb6da5f76b68a576327df87b4f247c688e0c6 Mon Sep 17 00:00:00 2001 From: janvrska <1644599+janvrska@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:07:54 +0200 Subject: [PATCH] feat: expire FUP entries as soon as the limited periods allow FUP entries were kept for 35 days of inactivity - the longest period the package supports - regardless of what the scope actually limits by. For a key with an unbounded key space that is the difference between holding a key for two days and for five weeks: a per-IP scope of {"minutely": 60, "daily": 5000} needs two days, and every source IP that is seen once mints its own key. The TTL is now derived from the scope being enforced and handed to the driver through contract.FUPTTLCacheDriverInterface, an optional addition to CacheDriverInterface - a driver that doesn't implement it keeps being used through IncrementFUPEntry, with constants.FUPEntryTTL as before, so this is not a breaking change. Both bundled drivers implement it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 11 +++++ README.md | 4 +- auth/cache/memory.go | 12 +++-- auth/cache/memory_test.go | 27 +++++++++++ auth/cache/redis.go | 6 ++- auth/cache/redis_test.go | 34 ++++++++++++++ auth/constants/main.go | 29 ++++++++++-- auth/constants/main_test.go | 30 +++++++++++++ auth/contract/cache.go | 20 ++++++++- auth/contract/entity.go | 22 +++++++++ auth/contract/entity_test.go | 70 +++++++++++++++++++++++++++++ auth/fup/main.go | 13 +++++- auth/fup/main_test.go | 87 ++++++++++++++++++++++++++++++++++++ auth/security/main_test.go | 8 +++- 14 files changed, 361 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f98ce0..9c639eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,17 @@ Changelog ==================================== +v3.1.0 +------------ + +### Added + +- `contract.FUPTTLCacheDriverInterface`, an optional addition to `contract.CacheDriverInterface` with a single method, `IncrementFUPEntryWithTTL(key string, ttl time.Duration) (*FUPCacheEntry, *AuthError)`. The FUP checkers use it when the configured driver implements it, deriving the TTL from the scope being enforced (`contract.FUPScope.GetEntryTTL`), so an entry is kept only as long as the longest period the scope limits by needs it - two days for a scope limiting per minute and per day, instead of the 35 the monthly period needs. That bounds the memory a FUP key with an unbounded key space (per IP, per cookie) can occupy: at the 490 requests/minute a single unauthenticated source was measured at, the difference is roughly 5 GB and 300 MB of Redis. + + Nothing has to be implemented - this is not a breaking change. A driver that only implements `CacheDriverInterface` keeps being used through `IncrementFUPEntry`, with `constants.FUPEntryTTL` for every entry as before. **A custom driver that wraps a bundled one** (to add metrics, to inject failures) **has to override both increments**, though, or the embedded implementation stays in use for the TTL-aware one. + +- `constants.Period.GetEntryTTL` returns how long an entry has to survive inactivity to keep counting that period correctly, and `contract.FUPScope.GetEntryTTL(path)` the longest such TTL a given scope path needs. + v3.0.0 ------------ diff --git a/README.md b/README.md index 0b15874..cd04aa8 100644 --- a/README.md +++ b/README.md @@ -563,6 +563,8 @@ If you implement your own driver, `IncrementFUPEntry` is the method to be carefu - an increment that arrives **out of order** (its period was decided before another request that was stored first) must not reset a counter or move the stored timestamp backwards, - entries should **expire** after `constants.FUPEntryTTL` of inactivity, otherwise counters of one-off sources (per-IP, per-cookie) accumulate forever. +Optionally, implement `contract.FUPTTLCacheDriverInterface` as well. Its `IncrementFUPEntryWithTTL` is the same increment with a caller-supplied expiration, derived from the scope being enforced, so entries live only as long as the longest period the scope limits by actually needs (a scope limiting per minute and per day keeps them for two days instead of 35). A driver that doesn't implement it keeps `constants.FUPEntryTTL` for everything, which is correct, just less frugal. If your driver *wraps* one of the built-in ones, override both increments - otherwise the embedded implementation stays in use for the TTL-aware one. + ### With user registration: By default, user registration is disabled. If you don't already have registration process in place, you can enable built-in registration by setting `WithRegistration` to `true` in `User` configuration (see below). @@ -824,7 +826,7 @@ If no limit is reached, each response to a request that has limits configured wi {"hourly":{"limit":200,"used":3},"minutely":{"limit":10,"used":1},"weekly":{"limit":100,"used":46}} ``` -FUP counters are stored in the cache under a key derived from the FUP key (client id, user login, `anonymous`) and the value the checker limits by (path, IP, cookie). They are incremented atomically, and they expire after 35 days of inactivity (slightly more than the longest supported interval). +FUP counters are stored in the cache under a key derived from the FUP key (client id, user login, `anonymous`) and the value the checker limits by (path, IP, cookie). They are incremented atomically, and they expire after the longest period the scope limits by needs them - two days for a scope limiting per minute and per day, 35 days if it limits per month (slightly more than the longest supported interval). The expiration is refreshed on every increment, so an entry only has to outlive the gap between two requests sharing a period. This matters for keys with an unbounded key space: a per-IP scope of `{"minutely": 60, "daily": 5000}` holds a key for two days per source IP, not 35. ### With anonymous FUP limits: diff --git a/auth/cache/memory.go b/auth/cache/memory.go index eba9221..e267f2c 100644 --- a/auth/cache/memory.go +++ b/auth/cache/memory.go @@ -152,10 +152,10 @@ func (d *MemoryCacheDriver) getFUPEntry(entryKey string) *contract.FUPCacheEntry } } -func (d *MemoryCacheDriver) setFUPEntry(entryKey string, entry *contract.FUPCacheEntry) { +func (d *MemoryCacheDriver) setFUPEntry(entryKey string, entry *contract.FUPCacheEntry, ttl time.Duration) { d.fupMemory[entryKey] = MemoryCacheEntry[contract.FUPCacheEntry]{ Value: *detachFUPEntry(entry), - ExpireAt: time.Now().Add(constants.FUPEntryTTL), + ExpireAt: time.Now().Add(ttl), } } @@ -168,17 +168,21 @@ func (d *MemoryCacheDriver) GetFUPEntry(key string) (*contract.FUPCacheEntry, *c func (d *MemoryCacheDriver) SetFUPEntry(key string, entry *contract.FUPCacheEntry) *contract.AuthError { d.fupLock.Lock() defer d.fupLock.Unlock() - d.setFUPEntry(d.getPrefix(GroupTypeFUP)+key, entry) + d.setFUPEntry(d.getPrefix(GroupTypeFUP)+key, entry, constants.FUPEntryTTL) return nil } func (d *MemoryCacheDriver) IncrementFUPEntry(key string) (*contract.FUPCacheEntry, *contract.AuthError) { + return d.IncrementFUPEntryWithTTL(key, constants.FUPEntryTTL) +} + +func (d *MemoryCacheDriver) IncrementFUPEntryWithTTL(key string, ttl time.Duration) (*contract.FUPCacheEntry, *contract.AuthError) { d.fupLock.Lock() defer d.fupLock.Unlock() entryKey := d.getPrefix(GroupTypeFUP) + key entry := d.getFUPEntry(entryKey) entry.Increment() - d.setFUPEntry(entryKey, entry) + d.setFUPEntry(entryKey, entry, ttl) return entry, nil } diff --git a/auth/cache/memory_test.go b/auth/cache/memory_test.go index f038184..d0eeea7 100644 --- a/auth/cache/memory_test.go +++ b/auth/cache/memory_test.go @@ -130,3 +130,30 @@ func TestMemoryCacheDriver_FUPEntryExpiration(t *testing.T) { t.Error("the expired entry is still in memory") } } + +// TestMemoryCacheDriver_IncrementFUPEntryWithTTL covers that the memory driver honours the +// caller-supplied expiration as well, so both bundled drivers behave the same +func TestMemoryCacheDriver_IncrementFUPEntryWithTTL(t *testing.T) { + driver := newTestMemoryDriver(t) + ttl := constants.PeriodDaily.GetEntryTTL() + + if _, err := driver.IncrementFUPEntryWithTTL("key", ttl); nil != err { + t.Fatalf("IncrementFUPEntryWithTTL() error = %v", err) + } + stored, ok := driver.fupMemory["test:fup_key"] + if !ok { + t.Fatalf("fupMemory has no entry for the incremented key") + } + if want, got := time.Now().Add(ttl), stored.ExpireAt; got.Sub(want) > time.Minute || want.Sub(got) > time.Minute { + t.Errorf("ExpireAt = %v, want about %v", got, want) + } + + // the default is unchanged for callers going through the interface method + if _, err := driver.IncrementFUPEntry("other"); nil != err { + t.Fatalf("IncrementFUPEntry() error = %v", err) + } + stored = driver.fupMemory["test:fup_other"] + if want, got := time.Now().Add(constants.FUPEntryTTL), stored.ExpireAt; got.Sub(want) > time.Minute || want.Sub(got) > time.Minute { + t.Errorf("ExpireAt = %v, want about %v", got, want) + } +} diff --git a/auth/cache/redis.go b/auth/cache/redis.go index f6e189e..eeb02fc 100644 --- a/auth/cache/redis.go +++ b/auth/cache/redis.go @@ -285,10 +285,14 @@ func (d *RedisCacheDriver) SetFUPEntry(key string, entry *contract.FUPCacheEntry } func (d *RedisCacheDriver) IncrementFUPEntry(key string) (*contract.FUPCacheEntry, *contract.AuthError) { + return d.IncrementFUPEntryWithTTL(key, constants.FUPEntryTTL) +} + +func (d *RedisCacheDriver) IncrementFUPEntryWithTTL(key string, ttl time.Duration) (*contract.FUPCacheEntry, *contract.AuthError) { entryKey := d.getPrefix(GroupTypeFUP) + key updatedAt := time.Now() args := make([]any, 0, 2+len(constants.FUPScopePeriods)*3) - args = append(args, updatedAt.Format(time.RFC3339Nano), int(constants.FUPEntryTTL.Seconds())) + args = append(args, updatedAt.Format(time.RFC3339Nano), int(ttl.Seconds())) for _, period := range constants.FUPScopePeriods { from, to := period.GetTimestampBounds(updatedAt) args = append(args, string(period), from, to) diff --git a/auth/cache/redis_test.go b/auth/cache/redis_test.go index 203ca59..2533ea7 100644 --- a/auth/cache/redis_test.go +++ b/auth/cache/redis_test.go @@ -286,3 +286,37 @@ func TestRedisCacheDriver_IncrementFUPEntry_Concurrent(t *testing.T) { t.Errorf("GetFUPEntry() daily = %d, want %d", got, requests) } } + +// TestRedisCacheDriver_IncrementFUPEntryWithTTL_TTL covers the expiration the FUP checkers derive +// from the scope - a scope limiting nothing longer than a day must not keep per-IP keys for 35 days +func TestRedisCacheDriver_IncrementFUPEntryWithTTL_TTL(t *testing.T) { + driver, server := newTestRedisDriver(t) + ttl := constants.PeriodDaily.GetEntryTTL() + + entry, err := driver.IncrementFUPEntryWithTTL("key", ttl) + if nil != err { + t.Fatalf("IncrementFUPEntryWithTTL() error = %v", err) + } + if got := entry.GetUsed(constants.PeriodDaily); 1 != got { + t.Errorf("IncrementFUPEntryWithTTL() daily = %d, want 1", got) + } + if got := server.TTL("test:fup_key"); ttl != got { + t.Errorf("TTL() = %v, want %v", got, ttl) + } + + // the expiration is refreshed on every increment, so the entry only has to outlive the gap + // between two requests + server.FastForward(ttl / 2) + if _, err := driver.IncrementFUPEntryWithTTL("key", ttl); nil != err { + t.Fatalf("IncrementFUPEntryWithTTL() error = %v", err) + } + if got := server.TTL("test:fup_key"); ttl != got { + t.Errorf("TTL() = %v, want %v after a second increment", got, ttl) + } + + // while a gap longer than the TTL releases the key instead of keeping it around + server.FastForward(ttl + time.Minute) + if server.Exists("test:fup_key") { + t.Errorf("Exists() = true, want the entry to be released after %v of inactivity", ttl) + } +} diff --git a/auth/constants/main.go b/auth/constants/main.go index 7f4249c..49b7ea6 100644 --- a/auth/constants/main.go +++ b/auth/constants/main.go @@ -36,11 +36,34 @@ const ( ApiUser = "api-user" ) -// FUPEntryTTL is the expiration of FUP cache entries. It is slightly longer than the longest -// FUP period (monthly), so that counters that are no longer used (e.g. per-IP counters of -// one-off visitors) are released instead of growing unbounded. +// FUPEntryTTL is the expiration of FUP cache entries whose limited periods are not known to the +// caller. It is slightly longer than the longest FUP period (monthly), so that counters that are +// no longer used (e.g. per-IP counters of one-off visitors) are released instead of growing +// unbounded. const FUPEntryTTL = time.Hour * 24 * 35 +// GetEntryTTL returns how long a FUP cache entry has to survive inactivity to keep counting this +// period correctly - the period itself plus a margin, since the expiration is refreshed on every +// increment and the entry only has to outlive the gap between two requests that share a period. +// A scope that limits by nothing longer than a day therefore keeps its entries for two days +// instead of the 35 the monthly period needs, which matters for keys with an unbounded key space +// (per IP, per cookie). +func (p Period) GetEntryTTL() time.Duration { + switch p { + case PeriodMinutely: + return time.Hour + case PeriodHourly: + return time.Hour * 25 + case PeriodDaily: + return time.Hour * 24 * 2 + case PeriodWeekly: + return time.Hour * 24 * 8 + case PeriodMonthly: + return FUPEntryTTL + } + return FUPEntryTTL +} + var ScopeAccessibilityOptions = []ScopeAccessibility{ ScopeAccessibilityAccessible, ScopeAccessibilityForbidden, diff --git a/auth/constants/main_test.go b/auth/constants/main_test.go index c9dc70f..2e82c02 100644 --- a/auth/constants/main_test.go +++ b/auth/constants/main_test.go @@ -77,3 +77,33 @@ func TestPeriod_GetTimestampBounds_WeekBoundaries(t *testing.T) { }) } } + +// TestPeriod_GetEntryTTL covers that every period keeps a counter alive for longer than the period +// it counts - an entry expiring within its own period would reset the counter early and let more +// requests through than the limit allows +func TestPeriod_GetEntryTTL(t *testing.T) { + tests := []struct { + period Period + want time.Duration + atLeast time.Duration + }{ + {period: PeriodMinutely, want: time.Hour, atLeast: time.Minute}, + {period: PeriodHourly, want: time.Hour * 25, atLeast: time.Hour}, + {period: PeriodDaily, want: time.Hour * 24 * 2, atLeast: time.Hour * 24}, + {period: PeriodWeekly, want: time.Hour * 24 * 8, atLeast: time.Hour * 24 * 7}, + {period: PeriodMonthly, want: FUPEntryTTL, atLeast: time.Hour * 24 * 31}, + // an unknown period must not shorten the expiration + {period: Period("yearly"), want: FUPEntryTTL, atLeast: time.Hour * 24 * 31}, + } + for _, tt := range tests { + t.Run(string(tt.period), func(t *testing.T) { + got := tt.period.GetEntryTTL() + if tt.want != got { + t.Errorf("GetEntryTTL() = %v, want %v", got, tt.want) + } + if got <= tt.atLeast { + t.Errorf("GetEntryTTL() = %v, want longer than the period itself (%v)", got, tt.atLeast) + } + }) + } +} diff --git a/auth/contract/cache.go b/auth/contract/cache.go index 28fea4b..79de8af 100644 --- a/auth/contract/cache.go +++ b/auth/contract/cache.go @@ -25,9 +25,27 @@ type CacheDriverInterface interface { // arrives out of order, i.e. one that decided its period before an increment that was // stored first - it counts towards the newer period instead, // - expire entries after constants.FUPEntryTTL of inactivity, otherwise the counters of - // one-off sources (per IP, per cookie) accumulate forever, + // one-off sources (per IP, per cookie) accumulate forever - implement + // FUPTTLCacheDriverInterface as well to expire them as soon as the limited periods allow, // - return an entry that shares no state with what is stored, since the caller reads it // while other requests keep incrementing. IncrementFUPEntry(key string) (*FUPCacheEntry, *AuthError) InvalidateToken(token string) *AuthError } + +// FUPTTLCacheDriverInterface is an optional addition to CacheDriverInterface for drivers that can +// expire a FUP entry after a caller-supplied TTL. The FUP checkers derive it from the scope being +// enforced (FUPScope.GetEntryTTL), so an entry is kept only as long as the longest period the +// scope limits by needs it - a scope limiting per minute and per day keeps its entries for two +// days rather than the 35 the monthly period would need. That bounds the memory a FUP key with an +// unbounded key space (per IP, per cookie) can occupy. +// +// A driver that doesn't implement it is used through IncrementFUPEntry and keeps every entry for +// constants.FUPEntryTTL, which is correct, just less frugal. The bundled memory and Redis drivers +// implement it - a driver that wraps one of them (to add metrics, to inject failures) therefore +// has to override both increments, or the embedded implementation stays in use for this one. +type FUPTTLCacheDriverInterface interface { + // IncrementFUPEntryWithTTL behaves exactly like CacheDriverInterface.IncrementFUPEntry, except + // that the entry expires after the given TTL of inactivity instead of constants.FUPEntryTTL. + IncrementFUPEntryWithTTL(key string, ttl time.Duration) (*FUPCacheEntry, *AuthError) +} diff --git a/auth/contract/entity.go b/auth/contract/entity.go index 8fc3a0a..f9af833 100644 --- a/auth/contract/entity.go +++ b/auth/contract/entity.go @@ -202,6 +202,28 @@ func (s FUPScope) HasLimit(key string) bool { return false } +// GetEntryTTL returns how long the cache entry of the given FUP path (e.g. constants.FUPIPKey) +// has to survive inactivity for this scope to be enforced - the longest period the scope actually +// limits by, plus a margin. A path the scope doesn't limit at all falls back to +// constants.FUPEntryTTL, which covers every period. +func (s FUPScope) GetEntryTTL(path string) time.Duration { + var ttl time.Duration + for _, period := range constants.FUPScopePeriods { + limit := s.GetLimit(path + "." + string(period)) + if nil == limit || *limit < 0 { + // no limitation, so nothing has to be counted for this period + continue + } + if periodTTL := period.GetEntryTTL(); periodTTL > ttl { + ttl = periodTTL + } + } + if 0 == ttl { + return constants.FUPEntryTTL + } + return ttl +} + type OneOffToken struct { Value string `json:"token"` Expires time.Time `json:"expires"` diff --git a/auth/contract/entity_test.go b/auth/contract/entity_test.go index 05fe4c0..20af44f 100644 --- a/auth/contract/entity_test.go +++ b/auth/contract/entity_test.go @@ -5,6 +5,7 @@ import ( "regexp" "sync" "testing" + "time" ) func TestAccessScope_GetAccessibility(t *testing.T) { @@ -677,3 +678,72 @@ func TestFUPScope_GetLimit_HasLimit(t *testing.T) { }) } } + +// TestFUPScope_GetEntryTTL covers that an entry is kept exactly as long as the periods the scope +// limits by need it - the point of the whole thing is that a scope limiting nothing longer than a +// day doesn't keep per-IP keys for 35 days +func TestFUPScope_GetEntryTTL(t *testing.T) { + tests := []struct { + name string + scope FUPScope + path string + want time.Duration + }{ + { + // the live anonymous scope + name: "minutely and daily", + scope: FUPScope{constants.FUPIPKey: map[string]any{"minutely": 60, "daily": 5000}}, + path: constants.FUPIPKey, + want: constants.PeriodDaily.GetEntryTTL(), + }, + { + name: "the longest period wins", + scope: FUPScope{constants.FUPIPKey: map[string]any{"minutely": 60, "monthly": 100000}}, + path: constants.FUPIPKey, + want: constants.FUPEntryTTL, + }, + { + name: "minutely only", + scope: FUPScope{constants.FUPIPKey: map[string]any{"minutely": 60}}, + path: constants.FUPIPKey, + want: constants.PeriodMinutely.GetEntryTTL(), + }, + { + // a negative limit means no limitation, so nothing has to be counted for that period + name: "negative limits are not counted", + scope: FUPScope{constants.FUPIPKey: map[string]any{"daily": 500, "monthly": -1}}, + path: constants.FUPIPKey, + want: constants.PeriodDaily.GetEntryTTL(), + }, + { + // other paths must not extend this one + name: "another path is not counted", + scope: FUPScope{ + constants.FUPIPKey: map[string]any{"minutely": 60}, + constants.FUPCookieKey: map[string]any{"monthly": 1000}, + }, + path: constants.FUPIPKey, + want: constants.PeriodMinutely.GetEntryTTL(), + }, + { + // nothing is limited, so fall back to the expiration that covers every period + name: "path not limited", + scope: FUPScope{constants.FUPCookieKey: map[string]any{"daily": 500}}, + path: constants.FUPIPKey, + want: constants.FUPEntryTTL, + }, + { + name: "empty scope", + scope: FUPScope{}, + path: constants.FUPIPKey, + want: constants.FUPEntryTTL, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.scope.GetEntryTTL(tt.path); tt.want != got { + t.Errorf("GetEntryTTL() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/auth/fup/main.go b/auth/fup/main.go index 5b2d8cd..832cbb9 100644 --- a/auth/fup/main.go +++ b/auth/fup/main.go @@ -6,12 +6,23 @@ import ( "github.com/wernerdweight/api-auth-go/v3/auth/constants" "github.com/wernerdweight/api-auth-go/v3/auth/contract" "strings" + "time" ) +// incrementFUPEntry increments the entry, expiring it as soon as the enforced periods allow if the +// driver supports it (see contract.FUPTTLCacheDriverInterface); a driver that doesn't keeps every +// entry for constants.FUPEntryTTL. +func incrementFUPEntry(cacheDriver contract.CacheDriverInterface, cacheKey string, ttl time.Duration) (*contract.FUPCacheEntry, *contract.AuthError) { + if ttlDriver, ok := cacheDriver.(contract.FUPTTLCacheDriverInterface); ok { + return ttlDriver.IncrementFUPEntryWithTTL(cacheKey, ttl) + } + return cacheDriver.IncrementFUPEntry(cacheKey) +} + func checkLimits(scope *contract.FUPScope, key string, cacheId string, path string, cacheDriver contract.CacheDriverInterface) (map[constants.Period]contract.FUPLimits, *contract.FUPScopeLimits) { limits := make(map[constants.Period]contract.FUPLimits) cacheKey := fmt.Sprintf("%s_%s", key, strings.Replace(cacheId, "/", "-", -1)) - cacheEntry, err := cacheDriver.IncrementFUPEntry(cacheKey) + cacheEntry, err := incrementFUPEntry(cacheDriver, cacheKey, scope.GetEntryTTL(path)) if nil != err { return nil, &contract.FUPScopeLimits{ Error: err, diff --git a/auth/fup/main_test.go b/auth/fup/main_test.go index 2c399ee..a6fa8a3 100644 --- a/auth/fup/main_test.go +++ b/auth/fup/main_test.go @@ -5,6 +5,7 @@ import ( "github.com/wernerdweight/api-auth-go/v3/auth/contract" "reflect" "testing" + "time" ) func Test_mergeLimits(t *testing.T) { @@ -121,3 +122,89 @@ func Test_mergeLimits(t *testing.T) { }) } } + +// ttlDriver implements the optional contract.FUPTTLCacheDriverInterface and records what it was +// asked for. The embedded interface is nil - anything but the increment panics, which is the point +type ttlDriver struct { + contract.CacheDriverInterface + ttl time.Duration + withoutTTL bool +} + +func (d *ttlDriver) IncrementFUPEntryWithTTL(_ string, ttl time.Duration) (*contract.FUPCacheEntry, *contract.AuthError) { + d.ttl = ttl + return &contract.FUPCacheEntry{UpdatedAt: time.Now(), Used: map[constants.Period]int{constants.PeriodMinutely: 1}}, nil +} + +func (d *ttlDriver) IncrementFUPEntry(_ string) (*contract.FUPCacheEntry, *contract.AuthError) { + d.withoutTTL = true + return &contract.FUPCacheEntry{UpdatedAt: time.Now(), Used: map[constants.Period]int{constants.PeriodMinutely: 1}}, nil +} + +// legacyDriver is a driver that only implements contract.CacheDriverInterface, i.e. one written +// against v3.0.0 - it has to keep working, just without the shortened expiration +type legacyDriver struct { + contract.CacheDriverInterface + incremented bool +} + +func (d *legacyDriver) IncrementFUPEntry(_ string) (*contract.FUPCacheEntry, *contract.AuthError) { + d.incremented = true + return &contract.FUPCacheEntry{UpdatedAt: time.Now(), Used: map[constants.Period]int{constants.PeriodMinutely: 1}}, nil +} + +// Test_checkLimits_EntryTTL covers that the expiration handed to the cache comes from the scope +// being enforced, not from the longest period the package supports +func Test_checkLimits_EntryTTL(t *testing.T) { + tests := []struct { + name string + scope contract.FUPScope + want time.Duration + }{ + { + // the live anonymous scope: nothing longer than a day is limited, so a per-IP key + // must not be kept for the 35 days the monthly period needs + name: "minutely and daily", + scope: contract.FUPScope{constants.FUPIPKey: map[string]any{"minutely": 60, "daily": 5000}}, + want: constants.PeriodDaily.GetEntryTTL(), + }, + { + name: "monthly", + scope: contract.FUPScope{constants.FUPIPKey: map[string]any{"monthly": 100000}}, + want: constants.FUPEntryTTL, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + driver := &ttlDriver{} + scope := tt.scope + _, scopeLimits := checkLimits(&scope, constants.AnonymousFUPKey, "192.0.2.10", constants.FUPIPKey, driver) + if nil != scopeLimits { + t.Fatalf("checkLimits() = %+v, want no limits to be hit", scopeLimits) + } + if driver.withoutTTL { + t.Errorf("checkLimits() used IncrementFUPEntry, want the TTL-aware increment") + } + if tt.want != driver.ttl { + t.Errorf("checkLimits() ttl = %v, want %v", driver.ttl, tt.want) + } + }) + } +} + +// Test_checkLimits_LegacyDriver covers that a driver written against v3.0.0, which doesn't +// implement the optional TTL interface, is still used - it keeps constants.FUPEntryTTL of its own +func Test_checkLimits_LegacyDriver(t *testing.T) { + driver := &legacyDriver{} + scope := contract.FUPScope{constants.FUPIPKey: map[string]any{"minutely": 60}} + limits, scopeLimits := checkLimits(&scope, constants.AnonymousFUPKey, "192.0.2.10", constants.FUPIPKey, driver) + if nil != scopeLimits { + t.Fatalf("checkLimits() = %+v, want no limits to be hit", scopeLimits) + } + if !driver.incremented { + t.Errorf("checkLimits() did not increment through IncrementFUPEntry") + } + if got, ok := limits[constants.PeriodMinutely]; !ok || 60 != got.Limit || 1 != got.Used { + t.Errorf("checkLimits() minutely = %+v, want limit 60 used 1", got) + } +} diff --git a/auth/security/main_test.go b/auth/security/main_test.go index fa7bf4a..9a8272c 100644 --- a/auth/security/main_test.go +++ b/auth/security/main_test.go @@ -39,7 +39,9 @@ func (p brokenApiClientProvider) ProvideByApiKey(_ string) (contract.ApiClientIn return nil, contract.NewInternalError(contract.DatabaseError, nil) } -// brokenFUPCacheDriver can't count requests, e.g. because the cache is unreachable +// brokenFUPCacheDriver can't count requests, e.g. because the cache is unreachable. It overrides +// both increments - the embedded driver implements the optional contract.FUPTTLCacheDriverInterface +// too, and overriding only IncrementFUPEntry would leave the embedded one in use type brokenFUPCacheDriver struct { *cache.MemoryCacheDriver } @@ -48,6 +50,10 @@ func (d brokenFUPCacheDriver) IncrementFUPEntry(_ string) (*contract.FUPCacheEnt return nil, contract.NewInternalError(contract.CacheError, nil) } +func (d brokenFUPCacheDriver) IncrementFUPEntryWithTTL(_ string, _ time.Duration) (*contract.FUPCacheEntry, *contract.AuthError) { + return nil, contract.NewInternalError(contract.CacheError, nil) +} + // initAnonymousFUP configures the provider with a fresh cache, so that each test starts with // empty counters func initAnonymousFUP(scope contract.FUPScope) {