diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b21cac..a1e3292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,27 @@ All notable changes to HyperCache are recorded here. The format follows ### Added +- **Token-refresh visibility for the OIDC source.** Closes RFC 0003 open question 6: the + `WithOIDCClientCredentials` source now wraps its `oauth2.TokenSource` with a logger that emits one + `"oidc token rotated"` Info line per real rotation (expiry change), staying silent on cached returns. + Operators debugging "why are my requests suddenly 401?" now see token age in the structured log alongside + the other lifecycle events. The wrapper holds the `*Client` by reference rather than capturing + `c.logger` at construction time, so `WithLogger` applied AFTER `WithOIDCClientCredentials` still reaches + the rotation log surface. Three unit tests in [`pkg/client/oidc_logging_test.go`](pkg/client/oidc_logging_test.go) + cover the rotation-logs case, the cached-returns-stay-silent case, and the nil-Client defensive path. +- **`GET /v1/me/can` capability probe + `Client.Can(ctx, capability)` SDK method.** Closes RFC 0003 open + question 5: callers can now check "do I have write?" without the speculative-write pattern (try the + action, catch the 403). The server endpoint validates against a closed set of capability strings + (`cache.read` / `cache.write` / `cache.admin`); unknown values return 400 BAD_REQUEST so typos surface as + client errors rather than silently degrading to allowed=false. The SDK method mirrors this: + `(true, nil)` / `(false, nil)` for the allow/deny answers; `errors.Is(err, ErrBadRequest)` for the + spelling-mistake path. `Identity.HasCapability` added to [`pkg/httpauth/policy.go`](pkg/httpauth/policy.go) + as the single authoritative check used by both the server handler and the SDK. Three handler tests in + [`cmd/hypercache-server/me_test.go`](cmd/hypercache-server/me_test.go) cover allowed/denied/unknown; + three SDK tests in [`pkg/client/client_test.go`](pkg/client/client_test.go) cover the parallel surface. + OpenAPI spec ([`cmd/hypercache-server/openapi.yaml`](cmd/hypercache-server/openapi.yaml)) gains the + `/v1/me/can` operation + `CanResponse` schema. New "Probing a single capability with `Can`" and + "Token-refresh visibility" sections in [`docs/client-sdk.md`](docs/client-sdk.md). - **Chaos hooks for resilience testing (Phase 7).** New [`backend.WithDistChaos(*Chaos)`](pkg/backend/dist_chaos.go) option transparently wraps the dist transport with configurable fault injection — drop rate and latency injection, both with per-call probability rolls diff --git a/Makefile b/Makefile index a8b91c3..0cb90e0 100644 --- a/Makefile +++ b/Makefile @@ -216,6 +216,8 @@ docs-serve: docs-build PYENV_VERSION=mkdocs mkdocs serve pre-commit: + @eval "$$(pyenv init -)" && \ + pyenv activate pre-commit && \ pre-commit run -a trailing-whitespace && \ pre-commit run -a end-of-file-fixer && \ pre-commit run -a markdownlint && \ diff --git a/cmd/hypercache-server/main.go b/cmd/hypercache-server/main.go index 0dc122e..6801699 100644 --- a/cmd/hypercache-server/main.go +++ b/cmd/hypercache-server/main.go @@ -465,6 +465,7 @@ func registerClientRoutes(app *fiber.App, policy httpauth.Policy, nodeCtx *nodeC app.Delete("/v1/cache/:key", write, func(c fiber.Ctx) error { return handleDelete(c, nodeCtx) }) app.Get("/v1/owners/:key", read, func(c fiber.Ctx) error { return handleOwners(c, nodeCtx) }) app.Get("/v1/me", read, handleMe) + app.Get("/v1/me/can", read, handleCan) app.Post("/v1/cache/batch/get", read, func(c fiber.Ctx) error { return handleBatchGet(c, nodeCtx) }) app.Post("/v1/cache/batch/put", write, func(c fiber.Ctx) error { return handleBatchPut(c, nodeCtx) }) @@ -1335,6 +1336,74 @@ func handleMe(c fiber.Ctx) error { }) } +// canResponse is the body of GET /v1/me/can?capability=. +// `Allowed` is the discrimination result; `Capability` echoes the +// caller's input so log scraping ties allow/deny to the asked +// capability without parsing the query string again. +type canResponse struct { + Capability string `json:"capability"` + Allowed bool `json:"allowed"` +} + +// capability strings — closed set in the `cache.` namespace. +// Unknown values return 400 rather than silently false so callers +// detect typos instead of shipping broken authz logic to prod. +const ( + capabilityCacheRead = "cache.read" + capabilityCacheWrite = "cache.write" + capabilityCacheAdmin = "cache.admin" +) + +// isKnownCapability reports whether s is one of the three +// recognized capability strings. Switch-based so a future +// capability is one named const + one case. +func isKnownCapability(s string) bool { + switch s { + case capabilityCacheRead, capabilityCacheWrite, capabilityCacheAdmin: + return true + default: + return false + } +} + +// handleCan implements GET /v1/me/can?capability=cache.write — +// per-capability authorization probe. Caller passes a capability +// string; the response says whether the resolved identity holds +// it. Cheaper than the speculative-write pattern (try the write, +// catch the 403), and stable across future scope-to-capability +// refactors (clients key off the capability string, not the +// internal scope shape). +// +// Requires the `read` scope — same threshold as /v1/me. Unknown +// capability values fail BAD_REQUEST so typos don't silently +// answer "not allowed" when the real issue is the caller's +// spelling. +func handleCan(c fiber.Ctx) error { + capability := c.Query("capability") + if capability == "" { + return jsonErr(c, fiber.StatusBadRequest, codeBadRequest, "missing 'capability' query parameter") + } + + if !isKnownCapability(capability) { + return jsonErr(c, fiber.StatusBadRequest, codeBadRequest, "unknown capability '"+capability+"'") + } + + identity, ok := c.Locals(httpauth.IdentityKey).(httpauth.Identity) + if !ok { + return jsonErr( + c, + fiber.StatusInternalServerError, + codeInternal, + "identity not resolved by middleware (wiring bug)", + ) + } + + return c.JSON(canResponse{ + Capability: capability, + Allowed: identity.HasCapability(capability), + }) +} + func main() { os.Exit(run()) } // run is the testable main body — separated so deferred cleanup diff --git a/cmd/hypercache-server/me_test.go b/cmd/hypercache-server/me_test.go index ceec481..39dd650 100644 --- a/cmd/hypercache-server/me_test.go +++ b/cmd/hypercache-server/me_test.go @@ -35,7 +35,7 @@ func TestHandleMe_BodyShape(t *testing.T) { want: meResponse{ ID: "ops-readonly", Scopes: []string{"read"}, - Capabilities: []string{"cache.read"}, + Capabilities: []string{capabilityCacheRead}, }, }, { @@ -47,7 +47,7 @@ func TestHandleMe_BodyShape(t *testing.T) { want: meResponse{ ID: "ops-rw", Scopes: []string{"read", "write"}, - Capabilities: []string{"cache.read", "cache.write"}, + Capabilities: []string{capabilityCacheRead, capabilityCacheWrite}, }, }, { @@ -59,7 +59,7 @@ func TestHandleMe_BodyShape(t *testing.T) { want: meResponse{ ID: "anonymous", Scopes: []string{"read", "write", "admin"}, - Capabilities: []string{"cache.read", "cache.write", "cache.admin"}, + Capabilities: []string{capabilityCacheRead, capabilityCacheWrite, capabilityCacheAdmin}, }, }, } @@ -172,3 +172,144 @@ func TestHandleMe_MissingLocals(t *testing.T) { t.Fatalf("status: got %d, want 500", resp.StatusCode) } } + +// TestHandleCan_AllowedAndDenied pins the canonical happy paths: +// an identity holding a capability gets allowed=true; the same +// identity probed against a capability it lacks gets allowed=false. +// Both produce 200 — "allowed=false" is a successful probe, not +// an authorization failure. +func TestHandleCan_AllowedAndDenied(t *testing.T) { + t.Parallel() + + readWriteIdentity := httpauth.Identity{ + ID: "ops-rw", + Scopes: []httpauth.Scope{httpauth.ScopeRead, httpauth.ScopeWrite}, + } + + tests := []struct { + name string + capability string + wantAllowed bool + }{ + {"read holder asks for read", capabilityCacheRead, true}, + {"rw holder asks for write", capabilityCacheWrite, true}, + {"rw holder asks for admin", capabilityCacheAdmin, false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + body := callCanWithIdentity(t, readWriteIdentity, tc.capability) + if body.Capability != tc.capability { + t.Errorf("capability echo: got %q, want %q", body.Capability, tc.capability) + } + + if body.Allowed != tc.wantAllowed { + t.Errorf("allowed: got %v, want %v", body.Allowed, tc.wantAllowed) + } + }) + } +} + +// TestHandleCan_MissingCapabilityParam pins the input-validation +// posture: a request without the `capability` query param fails +// 400 with the canonical error envelope. We don't silently +// default to allowed=false — that would let typos pass as +// "you can't do it" when the real issue is the missing argument. +func TestHandleCan_MissingCapabilityParam(t *testing.T) { + t.Parallel() + + app := fiber.New() + app.Use(func(c fiber.Ctx) error { + c.Locals(httpauth.IdentityKey, httpauth.Identity{ID: "x", Scopes: []httpauth.Scope{httpauth.ScopeRead}}) + + return c.Next() + }) + app.Get("/v1/me/can", handleCan) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/me/can", strings.NewReader("")) + + resp, err := app.Test(req) + if err != nil { + t.Fatalf("app.Test: %v", err) + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status: got %d, want 400", resp.StatusCode) + } +} + +// TestHandleCan_UnknownCapability pins that unrecognized +// capability strings fail 400, not silently allowed=false. A +// typo like `cache.reaad` should surface as a client error so +// the caller fixes their code rather than shipping a broken +// authz check. +func TestHandleCan_UnknownCapability(t *testing.T) { + t.Parallel() + + app := fiber.New() + app.Use(func(c fiber.Ctx) error { + c.Locals(httpauth.IdentityKey, httpauth.Identity{ + ID: "x", + Scopes: []httpauth.Scope{httpauth.ScopeRead, httpauth.ScopeWrite, httpauth.ScopeAdmin}, + }) + + return c.Next() + }) + app.Get("/v1/me/can", handleCan) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, + "/v1/me/can?capability=cache.reaad", strings.NewReader("")) + + resp, err := app.Test(req) + if err != nil { + t.Fatalf("app.Test: %v", err) + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status: got %d, want 400 (unknown capability)", resp.StatusCode) + } +} + +// callCanWithIdentity drives /v1/me/can with a pre-populated +// IdentityKey local. Returns the decoded canResponse; failed +// status / decode trips the test fatally. +func callCanWithIdentity(t *testing.T, identity httpauth.Identity, capability string) canResponse { + t.Helper() + + app := fiber.New() + app.Use(func(c fiber.Ctx) error { + c.Locals(httpauth.IdentityKey, identity) + + return c.Next() + }) + app.Get("/v1/me/can", handleCan) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, + "/v1/me/can?capability="+capability, strings.NewReader("")) + + resp, err := app.Test(req) + if err != nil { + t.Fatalf("app.Test: %v", err) + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status: got %d, want 200", resp.StatusCode) + } + + var got canResponse + + err = json.NewDecoder(resp.Body).Decode(&got) + if err != nil { + t.Fatalf("decode body: %v", err) + } + + return got +} diff --git a/cmd/hypercache-server/openapi.yaml b/cmd/hypercache-server/openapi.yaml index d8f0407..f9d6dfb 100644 --- a/cmd/hypercache-server/openapi.yaml +++ b/cmd/hypercache-server/openapi.yaml @@ -274,6 +274,43 @@ paths: $ref: "#/components/schemas/IdentityResponse" "401": { $ref: "#/components/responses/Unauthorized" } + /v1/me/can: + get: + operationId: canPerform + tags: [ meta ] + summary: Per-capability authorization probe. + description: | + Returns whether the resolved identity holds the requested + capability. Cheaper than the speculative-write pattern + (try the action, catch the 403) and stable across future + scope-to-capability refactors — clients should key off the + capability string, not the internal scope shape. + + Unknown capability values return 400 BAD_REQUEST so typos + don't silently answer "not allowed" when the real issue + is the caller's spelling. + + Requires the `read` scope — same threshold as `/v1/me`. + parameters: + - in: query + name: capability + required: true + schema: + type: string + enum: [ cache.read, cache.write, cache.admin ] + description: | + The capability string to probe. Must be one of the + three values in the closed `cache.*` namespace. + responses: + "200": + description: Probe result. + content: + application/json: + schema: + $ref: "#/components/schemas/CanResponse" + "400": { $ref: "#/components/responses/BadRequest" } + "401": { $ref: "#/components/responses/Unauthorized" } + /v1/cache/batch/get: post: operationId: batchGet @@ -522,6 +559,19 @@ components: items: type: string + CanResponse: + type: object + required: [ capability, allowed ] + properties: + capability: + type: string + description: Echoes the queried capability string. + allowed: + type: boolean + description: | + True when the resolved identity holds the requested + capability; false otherwise. + ItemEnvelope: type: object required: [ key, value, value_encoding, version, node, owners ] diff --git a/cmd/hypercache-server/openapi_test.go b/cmd/hypercache-server/openapi_test.go index 3a87ccd..8949514 100644 --- a/cmd/hypercache-server/openapi_test.go +++ b/cmd/hypercache-server/openapi_test.go @@ -97,6 +97,7 @@ func declaredMethodsForPath() map[string]map[string]struct{} { "/v1/cache/:key": {fiber.MethodPut: {}, fiber.MethodGet: {}, fiber.MethodHead: {}, fiber.MethodDelete: {}}, "/v1/owners/:key": {fiber.MethodGet: {}}, "/v1/me": {fiber.MethodGet: {}}, + "/v1/me/can": {fiber.MethodGet: {}}, "/v1/cache/batch/get": {fiber.MethodPost: {}}, "/v1/cache/batch/put": {fiber.MethodPost: {}}, "/v1/cache/batch/delete": {fiber.MethodPost: {}}, diff --git a/cspell.config.yaml b/cspell.config.yaml index 5de36d5..8aa0189 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -215,6 +215,7 @@ words: - pyenv - pygments - pymdownx + - reaad - recvcheck - rediscluster - Redocly diff --git a/docs/client-sdk.md b/docs/client-sdk.md index d92af07..3e76af1 100644 --- a/docs/client-sdk.md +++ b/docs/client-sdk.md @@ -250,6 +250,7 @@ if errors.Is(err, client.ErrAllEndpointsFailed) { | `BatchGet(ctx, keys)` | `[]BatchGetResult, error` | Per-key `Found` flag; misses are not errors | | `BatchDelete(ctx, keys)` | `[]BatchDeleteResult, error` | Per-item `Err`; idempotent | | `Identity(ctx)` | `*Identity, error` | `ErrUnauthorized` if the token is invalid | +| `Can(ctx, capability)` | `bool, error` | `ErrBadRequest` on unknown capability strings | | `Endpoints()` | `[]string` | Current view (post-refresh) | | `RefreshTopology(ctx)` | `error` | Manual refresh — usually called by the loop | | `Close()` | `error` | Stops the refresh loop; idempotent | @@ -272,6 +273,54 @@ if !id.HasCapability("cache.write") { Prefer `HasCapability("cache.write")` over `slices.Contains(id.Scopes, "write")` — capability strings stay stable if a scope is later split across multiple capabilities, while raw scope checks break on the rename. +### Probing a single capability with `Can` + +When a caller just needs "does this credential have write?" — and not the full scopes/capabilities slice — +`Client.Can` is the focused probe: + +```go +canWrite, err := c.Can(ctx, "cache.write") +if err != nil { + return err +} +if !canWrite { + return fmt.Errorf("this credential cannot write to the cluster") +} +``` + +The method maps to `GET /v1/me/can?capability=`. Denial (`allowed=false`) returns `(false, nil)` — +a successful probe, not an error. Spelling mistakes (an unknown capability string) come back as +`errors.Is(err, ErrBadRequest)` so the typo surfaces rather than silently degrading to "I guess I can't". +Use this for at-startup gating; use `Identity` when you need the full picture. + +## Token-refresh visibility + +With `WithOIDCClientCredentials`, the underlying `oauth2/clientcredentials` source rotates tokens silently +before expiry. Without instrumentation, "why are my requests suddenly 401?" is a hard debug — by the time +the operator looks, the token's already been refreshed. + +The SDK wraps the source so every rotation surfaces as an Info log via `WithLogger`: + +```text +{"time":"...","level":"INFO","msg":"oidc token rotated","expires_at":"2026-05-12T15:42:01Z","token_type":"Bearer"} +``` + +One line per rotation — the wrapper compares the new token's `Expiry` against the previous one and only +emits when they differ. Cached returns (the typical happy path between rotations) stay silent. + +Apply `WithLogger` so the rotations are visible: + +```go +c, _ := client.New( + endpoints, + client.WithLogger(slog.Default()), + client.WithOIDCClientCredentials(cfg), +) +``` + +`WithLogger` order doesn't matter — the wrapper reads the client's logger at rotation time, not at +construction. Late-bound `WithLogger` calls still reach the OIDC log surface. + ## Batch operations The single-key methods (`Set`/`Get`/`Delete`) are one HTTP round-trip per call. For hot loops or fan-in diff --git a/hypercache-server b/hypercache-server index 212b26a..09d15a4 100755 Binary files a/hypercache-server and b/hypercache-server differ diff --git a/pkg/client/client.go b/pkg/client/client.go index dd53866..0527ef7 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -277,6 +277,49 @@ func (c *Client) Identity(ctx context.Context) (*Identity, error) { }, nil } +// Can probes whether the caller's resolved identity holds the +// given capability. Cheaper than the speculative-write pattern +// (try the action, catch 403) and stable across future +// scope-to-capability refactors — clients key off the capability +// string, not the internal scope shape. +// +// Returns (true, nil) when the cluster confirms the capability, +// (false, nil) when it confirms the capability is missing, and +// (false, err) when the probe itself failed (network, auth, +// unknown capability — `errors.Is(err, ErrBadRequest)` discriminates +// the spelling-mistake case). +// +// Pair with the canonical at-startup canary: +// +// can, err := c.Can(ctx, "cache.write") +// if err != nil { log.Fatal(err) } +// if !can { log.Fatal("this credential cannot write") } +func (c *Client) Can(ctx context.Context, capability string) (bool, error) { + path := "/v1/me/can?capability=" + url.QueryEscape(capability) + + resp, err := c.do(ctx, http.MethodGet, path, nil, nil) + if err != nil { + return false, err + } + defer closeBody(resp) + + if resp.StatusCode != http.StatusOK { + return false, classifyResponse(resp) + } + + var out struct { + Capability string `json:"capability"` + Allowed bool `json:"allowed"` + } + + decodeErr := json.NewDecoder(resp.Body).Decode(&out) + if decodeErr != nil { + return false, fmt.Errorf("decode /v1/me/can: %w", decodeErr) + } + + return out.Allowed, nil +} + // --- internal helpers used by options.go --- // bearerAuthTransport returns a RoundTripper that injects the diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 3a36044..cd2fd7e 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -634,3 +634,92 @@ func TestClient_New_OptionErrors(t *testing.T) { }) } } + +// TestClient_Can_Allowed pins the happy path: the server says +// allowed=true and the SDK surfaces it as `true, nil`. +func TestClient_Can_Allowed(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/me/can" { + http.Error(w, "wrong route", http.StatusNotFound) + + return + } + + w.Header().Set("Content-Type", "application/json") + + _ = json.NewEncoder(w).Encode(map[string]any{ + "capability": r.URL.Query().Get("capability"), + "allowed": true, + }) + })) + + t.Cleanup(srv.Close) + + c, _ := client.New([]string{srv.URL}) + t.Cleanup(func() { _ = c.Close() }) + + allowed, err := c.Can(context.Background(), "cache.write") + if err != nil { + t.Fatalf("Can: %v", err) + } + + if !allowed { + t.Errorf("Can(cache.write): got allowed=false, want true") + } +} + +// TestClient_Can_Denied pins the inverse: when the server says +// allowed=false, the SDK surfaces `false, nil` — denial is not +// an error. The caller decides whether the missing capability is +// fatal at the application layer. +func TestClient_Can_Denied(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + _ = json.NewEncoder(w).Encode(map[string]any{ + "capability": r.URL.Query().Get("capability"), + "allowed": false, + }) + })) + + t.Cleanup(srv.Close) + + c, _ := client.New([]string{srv.URL}) + t.Cleanup(func() { _ = c.Close() }) + + allowed, err := c.Can(context.Background(), "cache.admin") + if err != nil { + t.Fatalf("Can: %v", err) + } + + if allowed { + t.Errorf("Can(cache.admin): got allowed=true, want false (denial path)") + } +} + +// TestClient_Can_UnknownCapability pins the spelling-mistake +// discrimination: an unknown capability comes back as +// ErrBadRequest, not (false, nil). Callers writing +// `if allowed, _ := c.Can(ctx, "cache.reaad")` should see the +// typo surface rather than silently degrade to "I guess I can't". +func TestClient_Can_UnknownCapability(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + writeError(w, http.StatusBadRequest, "BAD_REQUEST", "unknown capability 'cache.reaad'") + })) + + t.Cleanup(srv.Close) + + c, _ := client.New([]string{srv.URL}) + t.Cleanup(func() { _ = c.Close() }) + + _, err := c.Can(context.Background(), "cache.reaad") + if !errors.Is(err, client.ErrBadRequest) { + t.Errorf("Can(unknown): want errors.Is(_, ErrBadRequest), got %v", err) + } +} diff --git a/pkg/client/oidc_logging.go b/pkg/client/oidc_logging.go new file mode 100644 index 0000000..977b946 --- /dev/null +++ b/pkg/client/oidc_logging.go @@ -0,0 +1,68 @@ +package client + +import ( + "fmt" + "log/slog" + "sync" + "time" + + "golang.org/x/oauth2" +) + +// loggingTokenSource wraps an oauth2.TokenSource and emits an Info +// log line each time the underlying source returns a token whose +// expiry differs from the previous one — i.e. when a real refresh +// happened, not when the cache simply returned the still-valid +// token from memory. +// +// This closes RFC 0003 open question "Zero token-refresh +// visibility": before this wrapper, the OIDC source rotated tokens +// silently and operators debugging "why are my requests suddenly +// 401?" had no way to correlate request failures with token age. +// Now every rotation surfaces through the client's WithLogger +// surface alongside the other lifecycle events. +// +// The Client pointer is held by reference (not its logger field) +// so a WithLogger applied AFTER WithOIDCClientCredentials still +// reaches this wrapper — c.logger is read at rotation time, not +// at construction. +type loggingTokenSource struct { + inner oauth2.TokenSource + client *Client + + mu sync.Mutex + lastExp time.Time +} + +func newLoggingTokenSource(inner oauth2.TokenSource, c *Client) oauth2.TokenSource { + return &loggingTokenSource{inner: inner, client: c} +} + +// Token implements oauth2.TokenSource. Delegates to the inner +// source and logs when the returned token's Expiry differs from +// the most-recently-observed one. Cached tokens (same Expiry) +// produce no log noise — every line in the log corresponds to a +// real IdP rotation. +func (l *loggingTokenSource) Token() (*oauth2.Token, error) { + tok, err := l.inner.Token() + if err != nil { + return nil, fmt.Errorf("oidc token source: %w", err) + } + + l.mu.Lock() + + rotated := l.lastExp.IsZero() || !l.lastExp.Equal(tok.Expiry) + + l.lastExp = tok.Expiry + l.mu.Unlock() + + if rotated && l.client != nil && l.client.logger != nil { + l.client.logger.Info( + "oidc token rotated", + slog.Time("expires_at", tok.Expiry), + slog.String("token_type", tok.TokenType), + ) + } + + return tok, nil +} diff --git a/pkg/client/oidc_logging_test.go b/pkg/client/oidc_logging_test.go new file mode 100644 index 0000000..ce43244 --- /dev/null +++ b/pkg/client/oidc_logging_test.go @@ -0,0 +1,155 @@ +package client + +import ( + "bytes" + "encoding/json" + "log/slog" + "strings" + "sync/atomic" + "testing" + "time" + + "golang.org/x/oauth2" +) + +// stubTokenSource returns successive tokens from a pre-built +// queue. Used to drive the loggingTokenSource through rotation +// scenarios without an actual IdP. +type stubTokenSource struct { + tokens []*oauth2.Token + calls atomic.Int64 +} + +func (s *stubTokenSource) Token() (*oauth2.Token, error) { + n := s.calls.Add(1) - 1 + if int(n) >= len(s.tokens) { + // Past the configured queue: return the last token + // repeatedly (mimics oauth2's cached-token behavior). + return s.tokens[len(s.tokens)-1], nil + } + + return s.tokens[n], nil +} + +// TestLoggingTokenSource_LogsOnRotation pins the canonical behavior: +// each time the underlying TokenSource returns a token whose +// Expiry differs from the previous one, the wrapper emits one +// "oidc token rotated" log line. +// +// Two distinct tokens → two log lines. No spam from cached +// returns (covered by the next test). +func TestLoggingTokenSource_LogsOnRotation(t *testing.T) { + t.Parallel() + + now := time.Now() + + stub := &stubTokenSource{ + tokens: []*oauth2.Token{ + {AccessToken: "tok-1", TokenType: "Bearer", Expiry: now.Add(time.Hour)}, + {AccessToken: "tok-2", TokenType: "Bearer", Expiry: now.Add(2 * time.Hour)}, + }, + } + + buf := &bytes.Buffer{} + c := &Client{logger: slog.New(slog.NewJSONHandler(buf, nil))} + + source := newLoggingTokenSource(stub, c) + + for range 2 { + _, err := source.Token() + if err != nil { + t.Fatalf("Token: %v", err) + } + } + + lines := splitLines(buf.String()) + if len(lines) != 2 { + t.Fatalf("want 2 log lines, got %d: %v", len(lines), lines) + } + + for i, line := range lines { + var rec map[string]any + + err := json.Unmarshal([]byte(line), &rec) + if err != nil { + t.Fatalf("line %d malformed: %v", i, err) + } + + if rec["msg"] != "oidc token rotated" { + t.Errorf("line %d msg: got %q, want %q", i, rec["msg"], "oidc token rotated") + } + } +} + +// TestLoggingTokenSource_NoLogOnCachedToken pins the inverse: +// when the underlying source returns the same Expiry (the typical +// oauth2/clientcredentials path where the cached token is still +// valid), the wrapper stays silent. This is the contract that +// keeps log volume tied to actual rotations. +func TestLoggingTokenSource_NoLogOnCachedToken(t *testing.T) { + t.Parallel() + + exp := time.Now().Add(time.Hour) + stub := &stubTokenSource{ + tokens: []*oauth2.Token{ + {AccessToken: "cached", TokenType: "Bearer", Expiry: exp}, + }, + } + + buf := &bytes.Buffer{} + c := &Client{logger: slog.New(slog.NewJSONHandler(buf, nil))} + + source := newLoggingTokenSource(stub, c) + + // 5 calls: first returns the token (rotation log fires), the + // remaining four return the same cached token (no log). + for range 5 { + _, err := source.Token() + if err != nil { + t.Fatalf("Token: %v", err) + } + } + + lines := splitLines(buf.String()) + if len(lines) != 1 { + t.Errorf("cached-token reuse should emit one rotation log, got %d:\n%s", + len(lines), buf.String()) + } +} + +// TestLoggingTokenSource_NilClientIsNoop documents the defensive +// path: even if the wrapper is given a nil Client (shouldn't +// happen via WithOIDCClientCredentials, but the constructor +// doesn't reject it), Token() must not panic. The rotation log +// is just skipped. +func TestLoggingTokenSource_NilClientIsNoop(t *testing.T) { + t.Parallel() + + stub := &stubTokenSource{ + tokens: []*oauth2.Token{ + {AccessToken: "t", TokenType: "Bearer", Expiry: time.Now().Add(time.Hour)}, + }, + } + + source := newLoggingTokenSource(stub, nil) + + tok, err := source.Token() + if err != nil { + t.Fatalf("Token with nil client: %v", err) + } + + if tok.AccessToken != "t" { + t.Errorf("got token %q, want t", tok.AccessToken) + } +} + +// splitLines is a small helper to split a JSON-log buffer on +// newlines, dropping the final empty entry from the trailing \n. +func splitLines(s string) []string { + parts := strings.Split(strings.TrimRight(s, "\n"), "\n") + if len(parts) == 1 && parts[0] == "" { + return nil + } + + return parts +} diff --git a/pkg/client/options.go b/pkg/client/options.go index 6b06dde..53a6d4f 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -109,7 +109,14 @@ func WithOIDCClientCredentials(cfg clientcredentials.Config) Option { baseCtx := contextWithBaseHTTP(base) - c.http = oauth2.NewClient(baseCtx, cfg.TokenSource(baseCtx)) + // Wrap the underlying TokenSource so rotations log via the + // client's slog.Logger. The wrapper holds a pointer to the + // Client so WithLogger applied AFTER this option still + // reaches the refresh log surface — c.logger is read at + // rotation time, not capture time. + source := newLoggingTokenSource(cfg.TokenSource(baseCtx), c) + + c.http = oauth2.NewClient(baseCtx, source) return nil } diff --git a/pkg/httpauth/policy.go b/pkg/httpauth/policy.go index cde246f..8bbc4bd 100644 --- a/pkg/httpauth/policy.go +++ b/pkg/httpauth/policy.go @@ -102,6 +102,15 @@ func (i Identity) Capabilities() []string { return out } +// HasCapability reports whether the identity carries the given +// capability string. Used by the /v1/me/can probe endpoint and by +// the client SDK's mirror method — both want a single +// authoritative check rather than reinventing the +// scope-prefix-to-capability mapping at each call site. +func (i Identity) HasCapability(name string) bool { + return slices.Contains(i.Capabilities(), name) +} + // TokenIdentity is one bearer-token grant in a Policy. The Token // field is the raw secret; never log it. ID is what shows up in // audit logs / Identity.ID after a successful match.