From b45af8ebbf67ef90e67b2ad701dddc4e5c7e645a Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Sun, 11 Oct 2026 15:19:55 +0200 Subject: [PATCH] fix(cabana): scope the token last_used_at throttle to the guard instance - The process-global debounce map was keyed by token ID, so a stamp from one test database suppressed a later write in another and turned TestBackendTokenPostgres red depending on order - Each compositeGuard now owns a stampThrottle; the backend guard and the ResolveBackend guard of one activation share it, so a single-app binary still writes last_used_at at most once a minute per token - TestTokenLastUsedThrottlePerGuard pins the per-guard isolation --- modules/cabana/api_tokens.go | 34 ++++++++++++++++------ modules/cabana/http.go | 2 ++ modules/cabana/token_guard.go | 6 ++-- modules/cabana/token_guard_test.go | 45 ++++++++++++++++++++++++++++-- modules/cabana/token_smoke_test.go | 2 +- 5 files changed, 74 insertions(+), 15 deletions(-) diff --git a/modules/cabana/api_tokens.go b/modules/cabana/api_tokens.go index 4af8acc..f6c6b7b 100644 --- a/modules/cabana/api_tokens.go +++ b/modules/cabana/api_tokens.go @@ -270,20 +270,36 @@ func (s gormTokenStore) StampLastUsed(ctx context.Context, id uint, now time.Tim ).Error } -// lastStamp gates last_used_at writes to at most once per minute per token id -// in this process; the SQL UPDATE is additionally conditional. -var lastStamp sync.Map // uint -> time.Time +// stampThrottle gates last_used_at writes to at most once per minute per token +// ID per guard instance; one instance serves a cabana activation (the backend +// guard and the ResolveBackend guard share it). The SQL UPDATE is additionally +// conditional. +type stampThrottle struct { + last sync.Map // uint -> time.Time +} -func stampLastUsed(ctx context.Context, store tokenStore, id uint, now time.Time) { +// due reports whether id may be stamped at now and records the stamp if so. +// A nil throttle never debounces. +func (t *stampThrottle) due(id uint, now time.Time) bool { + if t == nil { + return true + } + if v, ok := t.last.Load(id); ok { + if now.Sub(v.(time.Time)) < time.Minute { + return false + } + } + t.last.Store(id, now) + return true +} + +func stampLastUsed(ctx context.Context, stamps *stampThrottle, store tokenStore, id uint, now time.Time) { if store == nil || id == 0 { return } - if v, ok := lastStamp.Load(id); ok { - if now.Sub(v.(time.Time)) < time.Minute { - return - } + if !stamps.due(id, now) { + return } - lastStamp.Store(id, now) if err := store.StampLastUsed(ctx, id, now); err != nil { slog.Default().WarnContext(ctx, "cabana: token last_used_at stamp failed", "token_id", id, "error", err) } diff --git a/modules/cabana/http.go b/modules/cabana/http.go index e06e5e8..1e0832f 100644 --- a/modules/cabana/http.go +++ b/modules/cabana/http.go @@ -171,6 +171,8 @@ func Activate(app *backpack.App, plugins []party.Plugin) (*Routes, error) { if _, published := app.Lookup[*backendResolver](); !published { cookieLessJWT := bouncer.NewBackendJWTGuard(secret, users, bl, writeUnauthenticated) resolverGuard := newCompositeGuard(cookieLessJWT, store, users, reg) + // The admin middleware and ResolveBackend share one throttle per activation. + resolverGuard.stamps = guard.stamps if err := app.Publish(&backendResolver{guard: resolverGuard}); err != nil { return nil, fmt.Errorf("cabana: backend resolver: %w", err) } diff --git a/modules/cabana/token_guard.go b/modules/cabana/token_guard.go index e953cd2..f4f8ab2 100644 --- a/modules/cabana/token_guard.go +++ b/modules/cabana/token_guard.go @@ -46,6 +46,8 @@ type compositeGuard struct { store tokenStore users bouncer.UserProvider reg *Registry + // stamps debounces last_used_at writes for this guard (see stampThrottle). + stamps *stampThrottle } var ( @@ -54,7 +56,7 @@ var ( ) func newCompositeGuard(jwt bouncer.Guard, store tokenStore, users bouncer.UserProvider, reg *Registry) *compositeGuard { - return &compositeGuard{jwt: jwt, store: store, users: users, reg: reg} + return &compositeGuard{jwt: jwt, store: store, users: users, reg: reg, stamps: &stampThrottle{}} } func (g *compositeGuard) AuthenticateCredential(r *http.Request) (*bouncer.Principal, any, error) { @@ -115,7 +117,7 @@ func (g *compositeGuard) authenticateToken(r *http.Request, raw string) (*bounce return nil, nil, newScopeError() } principal.Scope = limit - stampLastUsed(r.Context(), g.store, row.ID, now) + stampLastUsed(r.Context(), g.stamps, g.store, row.ID, now) logTokenAuth(r, "success", principal.ID, row.ID) return principal, row, nil } diff --git a/modules/cabana/token_guard_test.go b/modules/cabana/token_guard_test.go index c16cea7..b1c8c5d 100644 --- a/modules/cabana/token_guard_test.go +++ b/modules/cabana/token_guard_test.go @@ -160,7 +160,6 @@ func TestBackendTokenGuard(t *testing.T) { ID: 99, BackendUserID: 1, Name: "t", TokenHash: hash2, TokenHint: hint2, Scopes: lagoon.Jsonable[[]string]{Data: []string{"demo"}, Valid: true}, }) - lastStamp.Delete(uint(99)) g2 := newCompositeGuard(jwt, errStore, users, reg) r = httptest.NewRequest(http.MethodGet, "/x", nil) r.Header.Set("Authorization", "Bearer "+plain2) @@ -183,7 +182,6 @@ func TestTokenLastUsedDebounce(t *testing.T) { ID: 42, BackendUserID: 1, Name: "t", TokenHash: hash, TokenHint: hint, Scopes: lagoon.Jsonable[[]string]{Data: []string{"demo"}, Valid: true}, }) - lastStamp.Delete(uint(42)) store.stampN = 0 r := httptest.NewRequest(http.MethodGet, "/x", nil) r.Header.Set("Authorization", "Bearer "+plain) @@ -196,7 +194,7 @@ func TestTokenLastUsedDebounce(t *testing.T) { if store.stampN != 1 { t.Fatalf("within minute stamps = %d", store.stampN) } - lastStamp.Store(uint(42), time.Now().UTC().Add(-2*time.Minute)) + guard.stamps.last.Store(uint(42), time.Now().UTC().Add(-2*time.Minute)) if _, _, err := guard.AuthenticateCredential(r); err != nil { t.Fatal(err) } @@ -205,6 +203,47 @@ func TestTokenLastUsedDebounce(t *testing.T) { } } +// TestTokenLastUsedThrottlePerGuard pins that the last_used_at debounce +// belongs to the guard: two guards over separate stores that both hold token +// ID 7 (fresh databases restart IDs) each stamp once. +func TestTokenLastUsedThrottlePerGuard(t *testing.T) { + reg := &Registry{tokenScopes: tracerCatalog()} + dev := &bouncer.Principal{ID: 1, Backend: true, PermissionGrants: map[string]bool{"acme.demo.access": true}} + users := stubUsers{byID: map[uint]*bouncer.Principal{1: dev}} + newSide := func() (*memTokenStore, *compositeGuard, *http.Request) { + store := &memTokenStore{byHash: map[string]*BackendAPIToken{}} + plain, hash, hint, err := mintAPIToken() + if err != nil { + t.Fatal(err) + } + store.put(&BackendAPIToken{ + ID: 7, BackendUserID: 1, Name: "t", TokenHash: hash, TokenHint: hint, + Scopes: lagoon.Jsonable[[]string]{Data: []string{"demo"}, Valid: true}, + }) + r := httptest.NewRequest(http.MethodGet, "/x", nil) + r.Header.Set("Authorization", "Bearer "+plain) + return store, newCompositeGuard(&stubJWTGuard{}, store, users, reg), r + } + storeA, guardA, reqA := newSide() + storeB, guardB, reqB := newSide() + + if _, _, err := guardA.AuthenticateCredential(reqA); err != nil { + t.Fatal(err) + } + if _, _, err := guardB.AuthenticateCredential(reqB); err != nil { + t.Fatal(err) + } + if storeA.stampN != 1 || storeB.stampN != 1 { + t.Fatalf("stamps A=%d B=%d, want 1 each", storeA.stampN, storeB.stampN) + } + if _, _, err := guardB.AuthenticateCredential(reqB); err != nil { + t.Fatal(err) + } + if storeB.stampN != 1 { + t.Fatalf("within-guard debounce: B stamps = %d, want 1", storeB.stampN) + } +} + func TestResolveBackend(t *testing.T) { cat := tracerCatalog() reg := &Registry{tokenScopes: cat} diff --git a/modules/cabana/token_smoke_test.go b/modules/cabana/token_smoke_test.go index 3c2dacf..e08840e 100644 --- a/modules/cabana/token_smoke_test.go +++ b/modules/cabana/token_smoke_test.go @@ -348,7 +348,7 @@ func TestTokenGatesSmoke(t *testing.T) { } // Stamp debounce: two auths within a minute → one store stamp. - lastStamp.Delete(uint(101)) + guard.stamps.last.Delete(uint(101)) store.stampN = 0 r = httptest.NewRequest(http.MethodGet, "/x", nil) r.Header.Set("Authorization", "Bearer "+demoTok)