From c0469e6741e7a20e5400b82d2912f7ea2630f016 Mon Sep 17 00:00:00 2001 From: Fred Amaral Date: Tue, 21 Jul 2026 09:43:38 -0300 Subject: [PATCH] feat(middleware): add authz cache, circuit breaker, and bounded retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every authorized request made a synchronous POST /v1/authorize with no cache, breaker, or retry, so an authz-service blip or outage amplified straight into the request path. The call was also built with http.NewRequest (no context), so a caller's deadline/cancellation never reached it. Add three opt-in resilience layers (all default-off, so default behavior is byte-for-byte unchanged) plus the context fix, composed as breaker(retry(httpCall(ctx))) with the cache checked before the breaker: - Prereq fix: build the authz request with http.NewRequestWithContext, and wrap each call in context.WithTimeout(ctx, AUTH_TIMEOUT) (default 30s = behavior-neutral). The caller's cancellation now aborts the call, and the deadline caps the retry budget. - TTL decision cache (AUTH_CACHE_TTL > 0) keyed by (sub, resource, action, product) — never the raw token. Bounded, sharded, no-dependency map with lazy expiry (no background goroutine to leak). get never returns an expired entry. Documented staleness/revocation-lag window = TTL. - Circuit breaker (AUTH_BREAKER_ENABLED, sony/gobreaker promoted to direct). On open it serves ONLY a fresh positive cache hit (checked before the breaker), otherwise denies — never a stale grant. - Bounded retry (AUTH_RETRY_MAX, cenkalti/backoff/v5) on TRANSIENT failures only (network/timeout/5xx); authoritative 401/403 are never retried. Every fallback — breaker open, retry exhausted, timeout — denies (fail closed, 403 / PermissionDenied). This complements #107: misconfig at construction -> 503 (never mounts open); runtime outage -> deny. Two triggers, both fail-closed. Stacks on #107 (PR #127): built on its Required fail-closed flag. Must merge after #127; rebase onto develop and retarget base to develop once #127 lands. Closes #108 Claude-Session: https://claude.ai/code/session_01RcQLK4qK1kbpvXWjuAR3Xh --- README.md | 20 ++ auth/middleware/decisioncache.go | 136 +++++++++ auth/middleware/middleware.go | 138 ++++----- auth/middleware/resilience.go | 278 +++++++++++++++++ auth/middleware/resilience_test.go | 470 +++++++++++++++++++++++++++++ go.mod | 4 +- 6 files changed, 966 insertions(+), 80 deletions(-) create mode 100644 auth/middleware/decisioncache.go create mode 100644 auth/middleware/resilience.go create mode 100644 auth/middleware/resilience_test.go diff --git a/README.md b/README.md index 29c14ae..bbed4c0 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,26 @@ AUTH_M2M_PRODUCT_FORWARD_ENABLED=false # fail-open behavior. Set it in security-sensitive deployments so a # missing/typo'd address cannot silently downgrade a protected service to open. AUTH_REQUIRED=false + +# Optional authorization resilience (all opt-in; defaults preserve prior behavior). +# Every fallback denies (fail closed) — no path serves a request on an outage. +# +# AUTH_TIMEOUT bounds each authorization round-trip with a per-request deadline +# (Go duration). Defaults to 30s (behavior-neutral). It also caps the retry budget. +AUTH_TIMEOUT=30s +# AUTH_CACHE_TTL enables a short-lived decision cache when > 0, keyed by +# (subject, resource, action, product) — never the token. Empty/0 disables it +# (default). Security tradeoff: a permission revocation takes up to the TTL to +# propagate, so keep it small (5–15s). It sheds load and, with the breaker, +# survives brief authz outages by serving fresh positive decisions. +AUTH_CACHE_TTL= +# AUTH_BREAKER_ENABLED opens a circuit breaker after sustained authz failures. +# While open it serves ONLY a fresh positive cache hit and otherwise denies; +# it never serves a stale decision. Defaults to disabled. +AUTH_BREAKER_ENABLED=false +# AUTH_RETRY_MAX retries only TRANSIENT failures (network/timeout/5xx) up to N +# times within AUTH_TIMEOUT; authoritative 401/403 are never retried. 0 disables. +AUTH_RETRY_MAX=0 ``` ### 2. Create a new instance of the middleware: diff --git a/auth/middleware/decisioncache.go b/auth/middleware/decisioncache.go new file mode 100644 index 0000000..8ff8ba8 --- /dev/null +++ b/auth/middleware/decisioncache.go @@ -0,0 +1,136 @@ +package middleware + +import ( + "hash/fnv" + "sync" + "time" +) + +// decisionCacheShards is the fixed shard count. Sharding spreads lock contention +// across the authorization hot path so a single mutex is not serialized on every +// authorized request. Fixed (not configurable) — 16 is ample for the expected +// concurrency and keeps the type dependency-free. +const decisionCacheShards = 16 + +// decisionCacheMaxPerShard bounds memory under a burst of distinct subjects. +// ponytail: soft cap with sweep-on-write + single eviction, not an LRU; the short +// TTL makes entries self-expire quickly, so this only guards a pathological +// cardinality spike. Raise it (or swap for an LRU) only if eviction pressure shows +// up in practice. +const decisionCacheMaxPerShard = 1024 + +// cacheKey identifies an authorization decision by the inputs that determine it — +// the exact fields sent to the authz service. It NEVER contains the raw token: two +// tokens for the same subject must share a decision, and a token must never become +// a cache key (it would leak into memory keyed by a secret). +type cacheKey struct { + sub string + resource string + action string + product string +} + +// cacheEntry is a cached authorization decision with its expiry. +type cacheEntry struct { + authorized bool + expiresAt time.Time +} + +type cacheShard struct { + mu sync.Mutex + entries map[cacheKey]cacheEntry +} + +// decisionCache is a bounded, sharded, TTL authorization-decision cache. Expiry is +// lazy (checked on read, swept on write) so there is no background goroutine to +// leak. get never returns an expired entry, which is what keeps the breaker-open +// fallback fail-closed: a stale grant is never served. +type decisionCache struct { + ttl time.Duration + shards [decisionCacheShards]*cacheShard +} + +// newDecisionCache builds an enabled cache with the given TTL. ttl must be > 0 +// (callers gate on that before constructing). +func newDecisionCache(ttl time.Duration) *decisionCache { + c := &decisionCache{ttl: ttl} + for i := range c.shards { + c.shards[i] = &cacheShard{entries: make(map[cacheKey]cacheEntry)} + } + + return c +} + +// shardFor selects the shard for a key by hashing its fields with a separator, so +// distinct field boundaries cannot collide (e.g. {"a","b"} vs {"ab",""}). +func (c *decisionCache) shardFor(k cacheKey) *cacheShard { + h := fnv.New32a() + _, _ = h.Write([]byte(k.sub + "\x00" + k.resource + "\x00" + k.action + "\x00" + k.product)) + + return c.shards[h.Sum32()%decisionCacheShards] +} + +// get returns the cached decision for k and whether a FRESH entry exists. An +// expired entry is treated as absent (and evicted); callers therefore never see a +// stale decision — critical for the breaker-open path, which must not serve +// expired grants. +func (c *decisionCache) get(k cacheKey) (authorized, ok bool) { + shard := c.shardFor(k) + + shard.mu.Lock() + defer shard.mu.Unlock() + + entry, found := shard.entries[k] + if !found { + return false, false + } + + if time.Now().After(entry.expiresAt) { + delete(shard.entries, k) + + return false, false + } + + return entry.authorized, true +} + +// set stores a decision for k with the cache TTL. When the shard is at its soft +// cap it first sweeps expired entries and, if still full, evicts a single entry so +// the cache stays bounded. +func (c *decisionCache) set(k cacheKey, authorized bool) { + shard := c.shardFor(k) + + shard.mu.Lock() + defer shard.mu.Unlock() + + if len(shard.entries) >= decisionCacheMaxPerShard { + evictShard(shard) + } + + shard.entries[k] = cacheEntry{authorized: authorized, expiresAt: time.Now().Add(c.ttl)} +} + +// evictShard drops expired entries; if none were expired it removes one arbitrary +// entry so an insert can proceed without unbounded growth. Caller holds shard.mu. +func evictShard(shard *cacheShard) { + now := time.Now() + evicted := false + + for key, entry := range shard.entries { + if now.After(entry.expiresAt) { + delete(shard.entries, key) + + evicted = true + } + } + + if evicted { + return + } + + for key := range shard.entries { + delete(shard.entries, key) + + return + } +} diff --git a/auth/middleware/middleware.go b/auth/middleware/middleware.go index f738b01..498a16c 100644 --- a/auth/middleware/middleware.go +++ b/auth/middleware/middleware.go @@ -24,6 +24,7 @@ import ( libHTTP "github.com/LerianStudio/lib-commons/v6/commons/net/http" "github.com/gofiber/fiber/v3" jwt "github.com/golang-jwt/jwt/v5" + "github.com/sony/gobreaker" ) type AuthClient struct { @@ -49,6 +50,29 @@ type AuthClient struct { // missing/typo'd address can no longer silently downgrade a protected service // to fully open. Required bool + + // timeout bounds each authorization round-trip via a per-request context + // deadline. Read once from AUTH_TIMEOUT; defaults to 30s (behavior-neutral, + // matching the prior client-wide HTTP timeout) when unset/invalid. It also caps + // the retry budget. + timeout time.Duration + + // cache, when non-nil, memoizes authorization decisions for a short TTL keyed by + // (sub, resource, action, product) — NEVER the raw token. Enabled by + // AUTH_CACHE_TTL > 0 (opt-in; nil = no caching, prior behavior). It trades a + // bounded revocation-propagation lag (= TTL) for load shedding and outage + // resilience. + cache *decisionCache + + // breaker, when non-nil, short-circuits the authorization call after sustained + // failures (opt-in via AUTH_BREAKER_ENABLED). While open it serves only a fresh + // positive cache hit and otherwise denies — it never fails open. + breaker *gobreaker.CircuitBreaker + + // retryMax is the maximum number of retries applied to TRANSIENT authorization + // failures (network/timeout/5xx). Read once from AUTH_RETRY_MAX; 0 disables + // retry (opt-in). Authoritative decisions (401/403) are never retried. + retryMax uint } type AuthResponse struct { @@ -184,22 +208,27 @@ func NewAuthClient(address string, enabled bool, logger *log.Logger) *AuthClient } } - forwardM2MProduct := os.Getenv("AUTH_M2M_PRODUCT_FORWARD_ENABLED") == "true" - required := os.Getenv("AUTH_REQUIRED") == "true" + // Build the client once with all env-derived config, then return it from every + // path below; the health check only logs, it never changes these fields. + c := &AuthClient{ + Address: address, + Enabled: enabled, + Logger: l, + ForwardM2MProduct: os.Getenv("AUTH_M2M_PRODUCT_FORWARD_ENABLED") == "true", + Required: os.Getenv("AUTH_REQUIRED") == "true", + timeout: parseAuthTimeout(), + cache: newDecisionCacheFromEnv(), + breaker: newBreakerFromEnv(), + retryMax: parseRetryMax(), + } if !enabled || address == "" { - if required { + if c.Required { logErrorf(context.Background(), l, "AUTH_REQUIRED is set but auth is disabled or address is empty: middleware will fail closed (refuse to serve)") } - return &AuthClient{ - Address: address, - Enabled: enabled, - Logger: l, - ForwardM2MProduct: forwardM2MProduct, - Required: required, - } + return c } client := sharedHTTPClient @@ -211,21 +240,21 @@ func NewAuthClient(address string, enabled bool, logger *log.Logger) *AuthClient if err != nil { logErrorf(context.Background(), l, failedToConnectMsg, err) - return &AuthClient{Address: address, Enabled: enabled, Logger: l, ForwardM2MProduct: forwardM2MProduct, Required: required} + return c } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { logErrorf(context.Background(), l, failedToConnectMsg, resp.Status) - return &AuthClient{Address: address, Enabled: enabled, Logger: l, ForwardM2MProduct: forwardM2MProduct, Required: required} + return c } body, err := io.ReadAll(resp.Body) if err != nil { logErrorf(context.Background(), l, "Failed to read response body: %v", err) - return &AuthClient{Address: address, Enabled: enabled, Logger: l, ForwardM2MProduct: forwardM2MProduct, Required: required} + return c } if string(body) == "healthy" { @@ -234,13 +263,7 @@ func NewAuthClient(address string, enabled bool, logger *log.Logger) *AuthClient logErrorf(context.Background(), l, failedToConnectMsg, string(body)) } - return &AuthClient{ - Address: address, - Enabled: enabled, - Logger: l, - ForwardM2MProduct: forwardM2MProduct, - Required: required, - } + return c } // canAuthorize reports whether the client is able to perform an authorization @@ -398,7 +421,11 @@ func (auth *AuthClient) checkAuthorization(ctx context.Context, product, resourc attribute.String("app.request.request_id", reqID), ) - client := sharedHTTPClient + // Per-request deadline: propagate the caller's cancellation/deadline to the + // authz call (the request was previously built without a context, so an upstream + // cancel could not abort it) and cap the retry budget. Defaults to 30s. + ctx, cancel := context.WithTimeout(ctx, auth.requestTimeout()) + defer cancel() token, _, err := new(jwt.Parser).ParseUnverified(accessToken, jwt.MapClaims{}) if err != nil { @@ -453,66 +480,21 @@ func (auth *AuthClient) checkAuthorization(ctx context.Context, product, resourc return false, http.StatusInternalServerError, err } - req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/v1/authorize", auth.Address), bytes.NewBuffer(requestBodyJSON)) - if err != nil { - logErrorf(ctx, auth.Logger, "Failed to create request: %v", err) - - tracing.HandleSpanError(span, "Failed to create request", err) + // Cache key = exactly the authz request inputs (never the raw token). product is + // the value actually forwarded ("" when not forwarded), so the key matches the + // decision the authz service made. + key := cacheKey{sub: sub, resource: resource, action: action, product: requestBody["product"]} - return false, http.StatusInternalServerError, fmt.Errorf("failed to create request: %w", err) - } - - tracing.InjectHTTPContext(ctx, req.Header) - - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", accessToken) - - resp, err := client.Do(req) - if err != nil { - logErrorf(ctx, auth.Logger, "Failed to make request: %v", err) - - tracing.HandleSpanError(span, "Failed to make request", err) - - return false, http.StatusInternalServerError, fmt.Errorf("failed to make request: %w", err) - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - if err != nil { - logErrorf(ctx, auth.Logger, "Failed to read response body: %v", err) - - tracing.HandleSpanError(span, "Failed to read response body", err) - - return false, http.StatusInternalServerError, fmt.Errorf("failed to read response body: %w", err) - } - - respError, err := unmarshalErrorResponse(body) - if err != nil { - logErrorf(ctx, auth.Logger, "Failed to unmarshal auth error response: %v", err) - - tracing.HandleSpanError(span, "Failed to unmarshal auth error response", err) - - return false, http.StatusInternalServerError, fmt.Errorf("failed to unmarshal auth error response: %w", err) - } - - if respError.Code != "" && resp.StatusCode != http.StatusInternalServerError { - logErrorf(ctx, auth.Logger, "Authorization request failed: %s", respError.Message) - - tracing.HandleSpanError(span, "Authorization request failed", respError) - - return false, resp.StatusCode, respError - } - - var response AuthResponse - if err := json.Unmarshal(body, &response); err != nil { - logErrorf(ctx, auth.Logger, "Failed to unmarshal response: %v", err) - - tracing.HandleSpanError(span, "Failed to unmarshal response", err) - - return false, http.StatusInternalServerError, fmt.Errorf("failed to unmarshal response: %w", err) + // A fresh cache hit (positive OR negative) short-circuits before the breaker, so + // the breaker only ever runs on a miss — its open state then denies (never + // serving a stale grant). + if auth.cache != nil { + if authorized, hit := auth.cache.get(key); hit { + return authorized, http.StatusOK, nil + } } - return response.Authorized, resp.StatusCode, nil + return auth.resolveAuthz(ctx, span, accessToken, requestBodyJSON, key) } // GetApplicationToken sends a POST request to the authorization service to get a token for the application. diff --git a/auth/middleware/resilience.go b/auth/middleware/resilience.go new file mode 100644 index 0000000..d3cbc89 --- /dev/null +++ b/auth/middleware/resilience.go @@ -0,0 +1,278 @@ +package middleware + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "strconv" + "strings" + "time" + + "github.com/LerianStudio/lib-observability/v2/tracing" + "github.com/cenkalti/backoff/v5" + "github.com/sony/gobreaker" + "go.opentelemetry.io/otel/trace" +) + +const ( + // defaultAuthTimeout bounds each authorization round-trip when AUTH_TIMEOUT is + // unset/invalid. 30s matches the prior client-wide HTTP timeout, so the default + // is behavior-neutral. + defaultAuthTimeout = 30 * time.Second + + // defaultBreakerMaxFailures is the consecutive-failure count that trips the + // breaker when AUTH_BREAKER_ENABLED is set. + defaultBreakerMaxFailures uint32 = 5 + + // defaultBreakerOpenTimeout is how long the breaker stays open before probing. + defaultBreakerOpenTimeout = 30 * time.Second +) + +// authzOutcome is the result of one authorization round-trip, decoupled from +// checkAuthorization's (bool, int, error) contract so the retry/breaker layer can +// distinguish an authoritative decision from a transient failure: +// - authErr != nil -> surface this error with statusCode (an authoritative coded +// deny, or an internal parse/marshal failure) — exactly what the pre-resilience +// code returned. +// - authErr == nil -> a clean decision: authorized at statusCode. +type authzOutcome struct { + authorized bool + statusCode int + authErr error +} + +// requestTimeout is the per-request authorization deadline, falling back to the +// default when unset (e.g. a struct-literal client in tests). +func (auth *AuthClient) requestTimeout() time.Duration { + if auth.timeout > 0 { + return auth.timeout + } + + return defaultAuthTimeout +} + +// resolveAuthz performs the authorization call through the configured resilience +// layers and maps the result to checkAuthorization's contract. Every +// non-authoritative outcome — transient failure exhausted, breaker open, timeout — +// denies (fail closed): it returns (false, 403, nil), the same "not authorized" +// path a normal denial takes, never failing open. A clean decision is cached (when +// the cache is enabled) before being returned. +func (auth *AuthClient) resolveAuthz(ctx context.Context, span trace.Span, accessToken string, body []byte, key cacheKey) (bool, int, error) { + outcome, err := auth.invokeAuthz(ctx, span, accessToken, body) + if err != nil { + logErrorf(ctx, auth.Logger, "Authorization unavailable, denying (fail closed): %v", err) + tracing.HandleSpanError(span, "Authorization unavailable, denying", err) + + return false, http.StatusForbidden, nil + } + + if outcome.authErr != nil { + return false, outcome.statusCode, outcome.authErr + } + + if auth.cache != nil { + auth.cache.set(key, outcome.authorized) + } + + return outcome.authorized, outcome.statusCode, nil +} + +// invokeAuthz runs the authorization call under the resilience layers. Composition +// is breaker( retry( doAuthorizeCall ) ): retry absorbs transient blips; the +// breaker counts a sustained failure only once per fully-failed retry sequence +// and, once open, short-circuits. +// +// When neither retry nor breaker is enabled it makes a single call and returns the +// outcome as-is (identical to the pre-resilience behavior — transient +// classification is irrelevant without a layer to act on it). Otherwise a non-nil +// returned error means "transient failure exhausted" or "breaker open"; the caller +// denies. A nil error means the outcome is authoritative. +func (auth *AuthClient) invokeAuthz(ctx context.Context, span trace.Span, accessToken string, body []byte) (authzOutcome, error) { + call := func() (authzOutcome, error) { + return auth.doAuthorizeCall(ctx, span, accessToken, body) + } + + if auth.breaker == nil && auth.retryMax == 0 { + outcome, _ := call() + + return outcome, nil + } + + run := call + if auth.retryMax > 0 { + run = func() (authzOutcome, error) { + return backoff.Retry(ctx, call, + backoff.WithMaxTries(1+auth.retryMax), + backoff.WithMaxElapsedTime(auth.requestTimeout()), + ) + } + } + + if auth.breaker == nil { + return run() + } + + res, err := auth.breaker.Execute(func() (any, error) { + outcome, opErr := run() + + return outcome, opErr + }) + + outcome, _ := res.(authzOutcome) + + return outcome, err +} + +// doAuthorizeCall performs one POST /v1/authorize and classifies the result. The +// authzOutcome mirrors the legacy (bool, int, error) semantics exactly; the second +// return is non-nil ONLY for a TRANSIENT failure — a network error, a context +// timeout, or a 5xx — which is what the retry and breaker layers act on. +// Authoritative decisions (2xx, 401, 403) return a nil transient error, so they +// are never retried and never trip the breaker. +func (auth *AuthClient) doAuthorizeCall(ctx context.Context, span trace.Span, accessToken string, body []byte) (authzOutcome, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, fmt.Sprintf("%s/v1/authorize", auth.Address), bytes.NewReader(body)) + if err != nil { + logErrorf(ctx, auth.Logger, "Failed to create request: %v", err) + tracing.HandleSpanError(span, "Failed to create request", err) + + return authzOutcome{statusCode: http.StatusInternalServerError, authErr: fmt.Errorf("failed to create request: %w", err)}, nil + } + + tracing.InjectHTTPContext(ctx, req.Header) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", accessToken) + + resp, err := sharedHTTPClient.Do(req) + if err != nil { + logErrorf(ctx, auth.Logger, "Failed to make request: %v", err) + tracing.HandleSpanError(span, "Failed to make request", err) + + wrapped := fmt.Errorf("failed to make request: %w", err) + + return authzOutcome{statusCode: http.StatusInternalServerError, authErr: wrapped}, wrapped + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + logErrorf(ctx, auth.Logger, "Failed to read response body: %v", err) + tracing.HandleSpanError(span, "Failed to read response body", err) + + wrapped := fmt.Errorf("failed to read response body: %w", err) + + return authzOutcome{statusCode: http.StatusInternalServerError, authErr: wrapped}, wrapped + } + + outcome := auth.classifyResponse(ctx, span, resp.StatusCode, respBody) + + if resp.StatusCode >= http.StatusInternalServerError { + // 5xx is transient for the resilience layer regardless of how it is surfaced. + return outcome, fmt.Errorf("authz service returned status %d", resp.StatusCode) + } + + return outcome, nil +} + +// classifyResponse converts an authz HTTP response (status + body) into an +// authzOutcome, reproducing the pre-resilience decision logic exactly: a coded +// error body is an authoritative deny at its status; otherwise the AuthResponse +// decides; unparseable bodies are internal errors. +func (auth *AuthClient) classifyResponse(ctx context.Context, span trace.Span, statusCode int, body []byte) authzOutcome { + respError, err := unmarshalErrorResponse(body) + if err != nil { + logErrorf(ctx, auth.Logger, "Failed to unmarshal auth error response: %v", err) + tracing.HandleSpanError(span, "Failed to unmarshal auth error response", err) + + return authzOutcome{statusCode: http.StatusInternalServerError, authErr: fmt.Errorf("failed to unmarshal auth error response: %w", err)} + } + + if respError.Code != "" && statusCode != http.StatusInternalServerError { + logErrorf(ctx, auth.Logger, "Authorization request failed: %s", respError.Message) + tracing.HandleSpanError(span, "Authorization request failed", respError) + + return authzOutcome{statusCode: statusCode, authErr: respError} + } + + var response AuthResponse + if err := json.Unmarshal(body, &response); err != nil { + logErrorf(ctx, auth.Logger, "Failed to unmarshal response: %v", err) + tracing.HandleSpanError(span, "Failed to unmarshal response", err) + + return authzOutcome{statusCode: http.StatusInternalServerError, authErr: fmt.Errorf("failed to unmarshal response: %w", err)} + } + + return authzOutcome{authorized: response.Authorized, statusCode: statusCode} +} + +// newAuthBreaker builds a circuit breaker that opens after maxFailures consecutive +// failures and stays open for openTimeout before probing with a single request. +func newAuthBreaker(maxFailures uint32, openTimeout time.Duration) *gobreaker.CircuitBreaker { + return gobreaker.NewCircuitBreaker(gobreaker.Settings{ + Name: "lib-auth-authorize", + MaxRequests: 1, + Timeout: openTimeout, + ReadyToTrip: func(counts gobreaker.Counts) bool { + return counts.ConsecutiveFailures >= maxFailures + }, + }) +} + +// parseAuthTimeout reads AUTH_TIMEOUT (a Go duration, e.g. "5s"), falling back to +// the behavior-neutral default when unset or invalid. +func parseAuthTimeout() time.Duration { + if v := strings.TrimSpace(os.Getenv("AUTH_TIMEOUT")); v != "" { + if d, err := time.ParseDuration(v); err == nil && d > 0 { + return d + } + } + + return defaultAuthTimeout +} + +// newDecisionCacheFromEnv builds the decision cache when AUTH_CACHE_TTL is a +// positive Go duration, and nil (disabled) otherwise. The cache trades a bounded +// revocation-propagation lag (up to the TTL) for load shedding and outage +// resilience — a documented security tradeoff, kept tight by a small TTL. +func newDecisionCacheFromEnv() *decisionCache { + v := strings.TrimSpace(os.Getenv("AUTH_CACHE_TTL")) + if v == "" { + return nil + } + + d, err := time.ParseDuration(v) + if err != nil || d <= 0 { + return nil + } + + return newDecisionCache(d) +} + +// newBreakerFromEnv builds the circuit breaker when AUTH_BREAKER_ENABLED=="true", +// and nil (disabled) otherwise. +func newBreakerFromEnv() *gobreaker.CircuitBreaker { + if os.Getenv("AUTH_BREAKER_ENABLED") != "true" { + return nil + } + + return newAuthBreaker(defaultBreakerMaxFailures, defaultBreakerOpenTimeout) +} + +// parseRetryMax reads AUTH_RETRY_MAX (the maximum number of retries applied to +// transient failures), returning 0 (disabled) when unset or invalid. +func parseRetryMax() uint { + v := strings.TrimSpace(os.Getenv("AUTH_RETRY_MAX")) + if v == "" { + return 0 + } + + n, err := strconv.Atoi(v) + if err != nil || n < 0 { + return 0 + } + + return uint(n) +} diff --git a/auth/middleware/resilience_test.go b/auth/middleware/resilience_test.go new file mode 100644 index 0000000..9edf14f --- /dev/null +++ b/auth/middleware/resilience_test.go @@ -0,0 +1,470 @@ +package middleware + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/LerianStudio/lib-observability/v2/log" + jwt "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// userToken builds a normal-user test token (HS256; this branch does not verify +// signatures, so any signing key is fine). +func userToken() string { + return createTestJWT(jwt.MapClaims{ + "type": "normal-user", + "owner": "acme-org", + "sub": "user-1", + }) +} + +// countingAuthServer returns a server that records how many /v1/authorize requests +// it received and responds per the supplied handler. +func countingAuthServer(t *testing.T, handler func(w http.ResponseWriter, r *http.Request, n int64)) (*httptest.Server, *atomic.Int64) { + t.Helper() + + var hits atomic.Int64 + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n := hits.Add(1) + handler(w, r, n) + })) + + t.Cleanup(server.Close) + + return server, &hits +} + +func writeAuthorized(w http.ResponseWriter, authorized bool) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(AuthResponse{Authorized: authorized}) +} + +// --------------------------------------------------------------------------- +// decisionCache (unit) +// --------------------------------------------------------------------------- + +func TestDecisionCache_SetGetFresh(t *testing.T) { + t.Parallel() + + c := newDecisionCache(time.Minute) + key := cacheKey{sub: "s", resource: "r", action: "a", product: "p"} + + c.set(key, true) + + authorized, ok := c.get(key) + require.True(t, ok) + assert.True(t, authorized) +} + +func TestDecisionCache_NegativeDecisionCached(t *testing.T) { + t.Parallel() + + c := newDecisionCache(time.Minute) + key := cacheKey{sub: "s", resource: "r", action: "a"} + + c.set(key, false) + + authorized, ok := c.get(key) + require.True(t, ok) + assert.False(t, authorized) +} + +func TestDecisionCache_ExpiredEntryNotReturned(t *testing.T) { + t.Parallel() + + c := newDecisionCache(15 * time.Millisecond) + key := cacheKey{sub: "s", resource: "r", action: "a"} + + c.set(key, true) + + time.Sleep(40 * time.Millisecond) + + _, ok := c.get(key) + assert.False(t, ok, "an expired entry must never be served (would be fail-open under an outage)") +} + +func TestDecisionCache_KeyFieldsDoNotCollide(t *testing.T) { + t.Parallel() + + c := newDecisionCache(time.Minute) + + c.set(cacheKey{sub: "a", resource: "b"}, true) + c.set(cacheKey{sub: "ab", resource: ""}, false) + + got1, ok1 := c.get(cacheKey{sub: "a", resource: "b"}) + got2, ok2 := c.get(cacheKey{sub: "ab", resource: ""}) + + require.True(t, ok1) + require.True(t, ok2) + assert.True(t, got1) + assert.False(t, got2) +} + +func TestDecisionCache_BoundedUnderManyKeys(t *testing.T) { + t.Parallel() + + c := newDecisionCache(time.Minute) + + // Insert far more distinct keys than a single shard's cap to exercise eviction. + total := decisionCacheShards * decisionCacheMaxPerShard * 2 + for i := 0; i < total; i++ { + c.set(cacheKey{sub: "s", resource: "r", action: "a", product: string(rune(i)) + "-" + time.Now().String()}, true) + } + + size := 0 + for _, shard := range c.shards { + shard.mu.Lock() + size += len(shard.entries) + shard.mu.Unlock() + } + + assert.LessOrEqual(t, size, decisionCacheShards*decisionCacheMaxPerShard, "cache must stay bounded") +} + +// --------------------------------------------------------------------------- +// Cache integration via checkAuthorization +// --------------------------------------------------------------------------- + +func TestCheckAuthorization_CacheHit_AvoidsSecondPost(t *testing.T) { + t.Parallel() + + server, hits := countingAuthServer(t, func(w http.ResponseWriter, _ *http.Request, _ int64) { + writeAuthorized(w, true) + }) + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + cache: newDecisionCache(time.Minute), + } + + for i := 0; i < 3; i++ { + authorized, statusCode, err := auth.checkAuthorization(context.Background(), "", "res", "read", userToken()) + require.NoError(t, err) + assert.True(t, authorized) + assert.Equal(t, http.StatusOK, statusCode) + } + + assert.Equal(t, int64(1), hits.Load(), "cache hits must avoid re-querying the authz service") +} + +func TestCheckAuthorization_CacheExpiry_RequeriesAuthz(t *testing.T) { + t.Parallel() + + server, hits := countingAuthServer(t, func(w http.ResponseWriter, _ *http.Request, _ int64) { + writeAuthorized(w, true) + }) + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + cache: newDecisionCache(15 * time.Millisecond), + } + + _, _, err := auth.checkAuthorization(context.Background(), "", "res", "read", userToken()) + require.NoError(t, err) + + time.Sleep(40 * time.Millisecond) + + _, _, err = auth.checkAuthorization(context.Background(), "", "res", "read", userToken()) + require.NoError(t, err) + + assert.Equal(t, int64(2), hits.Load(), "an expired cache entry must trigger a fresh authz query") +} + +func TestCheckAuthorization_NegativeCache_Served(t *testing.T) { + t.Parallel() + + server, hits := countingAuthServer(t, func(w http.ResponseWriter, _ *http.Request, _ int64) { + writeAuthorized(w, false) + }) + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + cache: newDecisionCache(time.Minute), + } + + for i := 0; i < 2; i++ { + authorized, _, err := auth.checkAuthorization(context.Background(), "", "res", "read", userToken()) + require.NoError(t, err) + assert.False(t, authorized) + } + + assert.Equal(t, int64(1), hits.Load(), "a cached denial is served without re-querying") +} + +// --------------------------------------------------------------------------- +// Retry +// --------------------------------------------------------------------------- + +func TestCheckAuthorization_Retry_On5xx_EventuallySucceeds(t *testing.T) { + t.Parallel() + + server, hits := countingAuthServer(t, func(w http.ResponseWriter, _ *http.Request, n int64) { + if n == 1 { + w.WriteHeader(http.StatusInternalServerError) + + return + } + + writeAuthorized(w, true) + }) + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + timeout: 2 * time.Second, + retryMax: 2, + } + + authorized, statusCode, err := auth.checkAuthorization(context.Background(), "", "res", "read", userToken()) + + require.NoError(t, err) + assert.True(t, authorized) + assert.Equal(t, http.StatusOK, statusCode) + assert.Equal(t, int64(2), hits.Load(), "a transient 5xx must be retried") +} + +func TestCheckAuthorization_Retry_NotOn403(t *testing.T) { + t.Parallel() + + server, hits := countingAuthServer(t, func(w http.ResponseWriter, _ *http.Request, _ int64) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + _ = json.NewEncoder(w).Encode(map[string]string{"code": "FORBIDDEN", "message": "no"}) + }) + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + timeout: 2 * time.Second, + retryMax: 3, + } + + authorized, statusCode, err := auth.checkAuthorization(context.Background(), "", "res", "write", userToken()) + + require.Error(t, err) + assert.False(t, authorized) + assert.Equal(t, http.StatusForbidden, statusCode) + assert.Equal(t, int64(1), hits.Load(), "an authoritative 403 must never be retried") +} + +// --------------------------------------------------------------------------- +// Circuit breaker +// --------------------------------------------------------------------------- + +func TestCheckAuthorization_Breaker_OpensAfterN_AndDenies(t *testing.T) { + t.Parallel() + + server, hits := countingAuthServer(t, func(w http.ResponseWriter, _ *http.Request, _ int64) { + w.WriteHeader(http.StatusInternalServerError) + }) + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + breaker: newAuthBreaker(2, time.Minute), + } + + // Two consecutive transient (5xx) failures trip the breaker. + for i := 0; i < 2; i++ { + authorized, statusCode, err := auth.checkAuthorization(context.Background(), "", "res", "read", userToken()) + require.NoError(t, err, "runtime outage denies without surfacing an error") + assert.False(t, authorized) + assert.Equal(t, http.StatusForbidden, statusCode) + } + + // Breaker now open: the next call is denied WITHOUT touching the authz service. + authorized, statusCode, err := auth.checkAuthorization(context.Background(), "", "res", "read", userToken()) + require.NoError(t, err) + assert.False(t, authorized) + assert.Equal(t, http.StatusForbidden, statusCode) + + assert.Equal(t, int64(2), hits.Load(), "an open breaker must short-circuit, not reach the authz service") +} + +func TestCheckAuthorization_BreakerOpen_ServesFreshPositiveCacheOnly(t *testing.T) { + t.Parallel() + + var failing atomic.Bool + + server, hits := countingAuthServer(t, func(w http.ResponseWriter, _ *http.Request, _ int64) { + if failing.Load() { + w.WriteHeader(http.StatusInternalServerError) + + return + } + + writeAuthorized(w, true) + }) + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + cache: newDecisionCache(time.Minute), + breaker: newAuthBreaker(2, time.Minute), + } + + // Phase 1: prime a positive decision for "resCached" while the service is healthy. + authorized, _, err := auth.checkAuthorization(context.Background(), "", "resCached", "read", userToken()) + require.NoError(t, err) + require.True(t, authorized) + + // Phase 2: service fails; trip the breaker via a different (uncached) key. + failing.Store(true) + + for i := 0; i < 2; i++ { + _, _, err = auth.checkAuthorization(context.Background(), "", "resTrip", "read", userToken()) + require.NoError(t, err) + } + + hitsAfterTrip := hits.Load() + + // Breaker open + fresh positive cache hit -> allow (served from cache, no network). + authorized, statusCode, err := auth.checkAuthorization(context.Background(), "", "resCached", "read", userToken()) + require.NoError(t, err) + assert.True(t, authorized, "a fresh positive cache hit is served even while the breaker is open") + assert.Equal(t, http.StatusOK, statusCode) + + // Breaker open + no cache -> deny. + authorized, statusCode, err = auth.checkAuthorization(context.Background(), "", "resUncached", "read", userToken()) + require.NoError(t, err) + assert.False(t, authorized, "with the breaker open and no fresh cache, the request is denied") + assert.Equal(t, http.StatusForbidden, statusCode) + + assert.Equal(t, hitsAfterTrip, hits.Load(), "neither the cache hit nor the open-breaker deny may reach the authz service") +} + +// --------------------------------------------------------------------------- +// Per-request context deadline (prerequisite bug fix) +// --------------------------------------------------------------------------- + +func TestCheckAuthorization_ContextTimeout_AbortsCall(t *testing.T) { + t.Parallel() + + // Server blocks well past the client's per-request deadline. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + select { + case <-r.Context().Done(): + case <-time.After(2 * time.Second): + } + + writeAuthorized(w, true) + })) + defer server.Close() + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + timeout: 50 * time.Millisecond, + } + + start := time.Now() + authorized, _, err := auth.checkAuthorization(context.Background(), "", "res", "read", userToken()) + elapsed := time.Since(start) + + require.Error(t, err, "the per-request deadline must abort the authz call") + assert.False(t, authorized) + assert.Less(t, elapsed, time.Second, "the call must abort at the deadline, not wait for the server") +} + +func TestCheckAuthorization_CallerCancellation_Propagates(t *testing.T) { + t.Parallel() + + // Proves NewRequestWithContext wiring: the CALLER's cancellation reaches the + // HTTP call (previously the request was built without a context). + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + select { + case <-r.Context().Done(): + case <-time.After(2 * time.Second): + } + + writeAuthorized(w, true) + })) + defer server.Close() + + auth := &AuthClient{ + Address: server.URL, + Enabled: true, + Logger: &testLogger{}, + timeout: 5 * time.Second, + } + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(50 * time.Millisecond) + cancel() + }() + + start := time.Now() + _, _, err := auth.checkAuthorization(ctx, "", "res", "read", userToken()) + elapsed := time.Since(start) + + require.Error(t, err) + assert.Less(t, elapsed, time.Second, "caller cancellation must abort the authz call") +} + +// --------------------------------------------------------------------------- +// NewAuthClient - resilience config wiring +// --------------------------------------------------------------------------- + +func TestNewAuthClient_ResilienceConfig(t *testing.T) { + // Cannot use t.Parallel(): subtests use t.Setenv. enabled=false returns early + // without any network call, exercising the config wiring in isolation. + logger := log.Logger(&testLogger{}) + + t.Run("defaults_are_behavior_neutral", func(t *testing.T) { + t.Setenv("AUTH_TIMEOUT", "") + t.Setenv("AUTH_CACHE_TTL", "") + t.Setenv("AUTH_BREAKER_ENABLED", "") + t.Setenv("AUTH_RETRY_MAX", "") + + client := NewAuthClient("", false, &logger) + assert.Equal(t, defaultAuthTimeout, client.timeout) + assert.Nil(t, client.cache) + assert.Nil(t, client.breaker) + assert.Equal(t, uint(0), client.retryMax) + }) + + t.Run("all_knobs_enabled", func(t *testing.T) { + t.Setenv("AUTH_TIMEOUT", "5s") + t.Setenv("AUTH_CACHE_TTL", "5s") + t.Setenv("AUTH_BREAKER_ENABLED", "true") + t.Setenv("AUTH_RETRY_MAX", "2") + + client := NewAuthClient("", false, &logger) + assert.Equal(t, 5*time.Second, client.timeout) + assert.NotNil(t, client.cache) + assert.NotNil(t, client.breaker) + assert.Equal(t, uint(2), client.retryMax) + }) + + t.Run("invalid_values_fall_back_to_disabled", func(t *testing.T) { + t.Setenv("AUTH_TIMEOUT", "not-a-duration") + t.Setenv("AUTH_CACHE_TTL", "0s") + t.Setenv("AUTH_RETRY_MAX", "-1") + + client := NewAuthClient("", false, &logger) + assert.Equal(t, defaultAuthTimeout, client.timeout) + assert.Nil(t, client.cache) + assert.Equal(t, uint(0), client.retryMax) + }) +} diff --git a/go.mod b/go.mod index ee5c4f7..043fadc 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/LerianStudio/lib-observability/v2 v2.0.0-beta.1 github.com/andybalholm/brotli v1.2.2 // indirect github.com/bxcodec/dbresolver/v2 v2.2.1 // indirect - github.com/cenkalti/backoff/v5 v5.0.3 // indirect + github.com/cenkalti/backoff/v5 v5.0.3 github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/gabriel-vasile/mimetype v1.4.13 // indirect @@ -49,7 +49,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/shirou/gopsutil v3.21.11+incompatible // indirect github.com/shopspring/decimal v1.4.0 // indirect - github.com/sony/gobreaker v1.0.0 // indirect + github.com/sony/gobreaker v1.0.0 github.com/tklauser/go-sysconf v0.4.0 // indirect github.com/tklauser/numcpus v0.12.0 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect