From 3d7dcc1143d471da743838b7ec2c302e980ccb76 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 2 May 2026 13:31:49 +0800 Subject: [PATCH 1/7] feat(jwksauth)!: align with upstream JWT_PRIVATE_CLAIM_PREFIX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Decode server-attested Domain/Project/ServiceAccount under a configurable prefix (default "extra"); add WithPrivateClaimPrefix option with format validation - Surface remaining caller-supplied payload keys via Claims.Extras and TokenInfo.Extra(key) - Remove the special-cased Tenant field and Tenant() helper; tenant is now a caller-supplied key under Extras - Precompute prefixed payload keys at construction to keep the verify hot path allocation-free - Migrate existing tests from bare to prefixed claim keys; add new e2e tests covering prefix configuration BREAKING CHANGE: Server-attested claims are now read from "_domain", "_project", "_service_account" (default prefix "extra") rather than bare keys; pair this SDK release with an AuthGate Server that includes upstream PR #182. Claims.Tenant and TokenInfo.Tenant() are removed — read tenant via TokenInfo.Extra("tenant"), e.g. v, _ := info.Extra("tenant"); s, _ := v.(string). Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 2 + jwksauth/README.md | 85 +++++++---- jwksauth/access_rule.go | 9 +- jwksauth/access_rule_test.go | 43 +++--- jwksauth/claims.go | 130 ++++++++++++---- jwksauth/claims_prefix_test.go | 271 +++++++++++++++++++++++++++++++++ jwksauth/doc.go | 24 ++- jwksauth/middleware_test.go | 72 ++++----- jwksauth/multi_verifier.go | 16 +- jwksauth/options.go | 79 +++++++++- jwksauth/verifier.go | 10 +- 11 files changed, 598 insertions(+), 143 deletions(-) create mode 100644 jwksauth/claims_prefix_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 34b6bbf..764e42d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,6 +8,8 @@ Go SDK for AuthGate — currently provides the `credstore` package for secure cr Module: `github.com/go-authgate/sdk-go` (Go 1.25+) +The `jwksauth` package's default private-claim prefix is `"extra"`, matching the upstream AuthGate `JWT_PRIVATE_CLAIM_PREFIX` default; deployments that override the server-side value must pass the same string via `jwksauth.WithPrivateClaimPrefix(...)`. + ## Common Commands ```bash diff --git a/jwksauth/README.md b/jwksauth/README.md index 871e709..fa30219 100644 --- a/jwksauth/README.md +++ b/jwksauth/README.md @@ -60,29 +60,52 @@ func profile(w http.ResponseWriter, r *http.Request) { } ``` -## Domain and Tenant hierarchy +## Server-attested private claims and the prefix -AuthGate partitions tokens along two dimensions: +AuthGate emits three private claims on every token: **Domain**, **Project**, +**ServiceAccount**. They appear in the payload under a configurable prefix +(default `extra`), so the JWT keys are `extra_domain`, `extra_project`, +`extra_service_account`. The SDK reads them out of the box. -- **Domain** (`domain` claim, e.g. `oa`, `swrd`, `hwrd`) is the top-level - partition. Every token has exactly one Domain. -- **Tenant** (`tenant` claim, e.g. `a76`, `a78`) is an *optional* sub-room - inside a Domain. Domains that have no sub-room concept simply omit the - claim, and the SDK exposes that as `info.Tenant() == ""`. +```json +{ + "iss": "https://auth.example.com", + "extra_domain": "oa", + "extra_project": "p1", + "extra_service_account": "sync-bot@oa.local" +} +``` -Both claims appear independently in the JWT payload: +If your AuthGate deployment overrides `JWT_PRIVATE_CLAIM_PREFIX`, pass the +same value to the verifier: -```json -// Domain-only token (Domain has no Tenant concept) -{ "iss": "https://auth.example.com", "domain": "oa" } +```go +v, err := jwksauth.NewVerifier(ctx, issuerURL, audience, + jwksauth.WithPrivateClaimPrefix("acme")) // reads acme_domain, acme_project, acme_service_account +``` + +Server and SDK must agree byte-for-byte. Reading with the wrong prefix +yields empty Domain / Project / ServiceAccount and (when `AccessRule` +covers those dimensions) fails closed. -// Domain + Tenant token (sub-room inside a Domain) -{ "iss": "https://auth.example.com", "domain": "oa", "tenant": "a76" } +### Caller-supplied keys (Extras) + +Any other non-standard payload keys — for example a caller-supplied +`tenant` — are surfaced on `Claims.Extras`. Read them with +`TokenInfo.Extra`: + +```go +if v, ok := info.Extra("tenant"); ok { + if s, ok := v.(string); ok { + // use the caller-supplied tenant value + _ = s + } +} ``` -`AccessRule` filters on Domain only; the optional Tenant value is exposed -on `TokenInfo` for application code to read but is not enforced at the -allowlist level. +`AccessRule` and the cross-issuer Domain pinning never look at Extras — +caller-supplied keys are not server-attested, so they cannot be used to +gate access. Apply your own checks in the handler when needed. ## Multiple issuers @@ -101,8 +124,8 @@ if err != nil { log.Fatal(err) } // domain codes ("oa" / "hwrd" / "swrd") this stops a compromised issuer // from minting tokens that claim a Domain owned by another issuer. // -// Tenants live entirely inside a Domain, so they are not part of the -// cross-issuer pinning encoding. +// Caller-supplied keys (surfaced via Claims.Extras) are not part of the +// cross-issuer pinning — only the server-attested Domain participates. if err := mv.SetIssuerDomains( "https://auth-a.example.com=oa,hwrd;https://auth-b.example.com=swrd", ); err != nil { @@ -126,17 +149,21 @@ mux.Handle("/api/admin", jwksauth.Middleware(mv, jwksauth.AccessRule{ ## AccessRule Per-route policy; an empty slice means "this dimension is not checked". - -| Field | Required claim | Match | Notes | -| ----------------- | ----------------- | --------- | ------------------------------------------------------------------------------- | -| `Scopes` | `scope` | space-set | Reports `403 insufficient_scope` and advertises the scope on `WWW-Authenticate` | -| `Domains` | `domain` | case-fold | SDK lower-cases the rule on registration | -| `ServiceAccounts` | `service_account` | exact | Case-sensitive | -| `Projects` | `project` | exact | Case-sensitive | - -The optional `tenant` claim is read from the JWT and exposed on -`TokenInfo.Tenant()` (case-folded) and `TokenInfo.Claims.Tenant` (raw), but -is not part of the allowlist surface. +The "Required claim" column shows the JWT payload key under the default +prefix (`extra`); under a custom prefix the keys become +`_domain` etc. + +| Field | Required claim | Match | Notes | +| ----------------- | ----------------------- | --------- | ------------------------------------------------------------------------------- | +| `Scopes` | `scope` | space-set | Reports `403 insufficient_scope` and advertises the scope on `WWW-Authenticate` | +| `Domains` | `extra_domain` | case-fold | SDK lower-cases the rule on registration | +| `ServiceAccounts` | `extra_service_account` | exact | Case-sensitive | +| `Projects` | `extra_project` | exact | Case-sensitive | + +Caller-supplied keys (anything else in the JWT payload that isn't a JWT/OIDC +standard claim) are not part of the allowlist surface. Read them from +`Claims.Extras` via `TokenInfo.Extra(key)` and apply your own logic in the +handler when needed. Allowlist mismatches return `401 invalid_token` (generic) so the allowlist itself is not probeable. The full reason is logged server-side via the diff --git a/jwksauth/access_rule.go b/jwksauth/access_rule.go index f6271ab..0f93e09 100644 --- a/jwksauth/access_rule.go +++ b/jwksauth/access_rule.go @@ -7,10 +7,11 @@ import ( ) // AccessRule is a per-route policy: the OAuth scopes the caller must hold -// plus optional allowlists for the domain / service_account / project -// claims AuthGate may emit. Filtering on the optional sub-room Tenant claim -// is intentionally out of scope at the rule level — Domains are the -// allowlist dimension. +// plus optional allowlists for the three server-attested private claims +// (Domain, ServiceAccount, Project) that AuthGate emits under the +// configured prefix. Caller-supplied keys surfaced via [Claims.Extras] do +// not participate in AccessRule comparisons; if you need to filter on a +// custom dimension, read it from Extras and check it in your handler. // // Semantics: // - An empty slice means "this dimension is not checked". diff --git a/jwksauth/access_rule_test.go b/jwksauth/access_rule_test.go index b6ea77d..c67bc66 100644 --- a/jwksauth/access_rule_test.go +++ b/jwksauth/access_rule_test.go @@ -2,11 +2,10 @@ package jwksauth import "testing" -func newInfo(domain, tenant, sa, project string) *TokenInfo { +func newInfo(domain, sa, project string) *TokenInfo { return &TokenInfo{ Claims: Claims{ Domain: domain, - Tenant: tenant, ServiceAccount: sa, Project: project, }, @@ -15,7 +14,7 @@ func newInfo(domain, tenant, sa, project string) *TokenInfo { func TestAccessRule_EmptyAllowsAll(t *testing.T) { rule := AccessRule{}.canonical() - if reason, ok := rule.checkClaims(newInfo("", "", "", "")); !ok { + if reason, ok := rule.checkClaims(newInfo("", "", "")); !ok { t.Errorf("empty rule should accept; reason=%q", reason) } } @@ -23,54 +22,56 @@ func TestAccessRule_EmptyAllowsAll(t *testing.T) { func TestAccessRule_DomainAllowlistCaseInsensitive(t *testing.T) { rule := AccessRule{Domains: []string{"OA", "HwRd"}}.canonical() - if _, ok := rule.checkClaims(newInfo("oa", "", "", "")); !ok { + if _, ok := rule.checkClaims(newInfo("oa", "", "")); !ok { t.Error("domain=oa should match (input was OA)") } - if _, ok := rule.checkClaims(newInfo("HWRD", "", "", "")); !ok { + if _, ok := rule.checkClaims(newInfo("HWRD", "", "")); !ok { t.Error("domain=HWRD should match (input was HwRd)") } - if _, ok := rule.checkClaims(newInfo("swrd", "", "", "")); ok { + if _, ok := rule.checkClaims(newInfo("swrd", "", "")); ok { t.Error("domain=swrd should not match") } } func TestAccessRule_FailClosedOnMissingClaim(t *testing.T) { rule := AccessRule{Domains: []string{"oa"}}.canonical() - if _, ok := rule.checkClaims(newInfo("", "", "", "")); ok { + if _, ok := rule.checkClaims(newInfo("", "", "")); ok { t.Error("missing domain should be rejected when allowlist is set") } } -// TestAccessRule_TenantNotFiltered pins the contract that the optional -// sub-room Tenant claim is intentionally not part of AccessRule. A token -// with Domain in the allowlist passes regardless of whether it carries a -// Tenant value. -func TestAccessRule_TenantNotFiltered(t *testing.T) { +// TestAccessRule_ExtrasNotFiltered pins the contract that caller-supplied +// keys surfaced via Claims.Extras are intentionally not part of AccessRule. +// A token with Domain in the allowlist passes regardless of which keys +// it carries in Extras. +func TestAccessRule_ExtrasNotFiltered(t *testing.T) { rule := AccessRule{Domains: []string{"oa"}}.canonical() - if _, ok := rule.checkClaims(newInfo("oa", "a76", "", "")); !ok { - t.Error("Domain match with Tenant present should accept") + withExtras := newInfo("oa", "", "") + withExtras.Claims.Extras = map[string]any{"tenant": "a76"} + if _, ok := rule.checkClaims(withExtras); !ok { + t.Error("Domain match with Extras present should accept") } - if _, ok := rule.checkClaims(newInfo("oa", "", "", "")); !ok { - t.Error("Domain match with Tenant absent should accept") + if _, ok := rule.checkClaims(newInfo("oa", "", "")); !ok { + t.Error("Domain match with Extras absent should accept") } } func TestAccessRule_ServiceAccountExactMatch(t *testing.T) { rule := AccessRule{ServiceAccounts: []string{"sync-bot@oa.local"}}.canonical() - if _, ok := rule.checkClaims(newInfo("", "", "sync-bot@oa.local", "")); !ok { + if _, ok := rule.checkClaims(newInfo("", "sync-bot@oa.local", "")); !ok { t.Error("exact match should accept") } - if _, ok := rule.checkClaims(newInfo("", "", "SYNC-BOT@OA.LOCAL", "")); ok { + if _, ok := rule.checkClaims(newInfo("", "SYNC-BOT@OA.LOCAL", "")); ok { t.Error("ServiceAccounts must be case-sensitive") } } func TestAccessRule_ProjectAllowlist(t *testing.T) { rule := AccessRule{Projects: []string{"admin-tools"}}.canonical() - if _, ok := rule.checkClaims(newInfo("", "", "", "admin-tools")); !ok { + if _, ok := rule.checkClaims(newInfo("", "", "admin-tools")); !ok { t.Error("project should match") } - if _, ok := rule.checkClaims(newInfo("", "", "", "other")); ok { + if _, ok := rule.checkClaims(newInfo("", "", "other")); ok { t.Error("project should not match") } } @@ -104,7 +105,7 @@ func TestAccessRule_CanonicalDropsEmpty(t *testing.T) { } // Token with missing claims must NOT pass the allowlists. - if _, ok := rule.checkClaims(newInfo("", "", "", "")); ok { + if _, ok := rule.checkClaims(newInfo("", "", "")); ok { t.Error("missing claims passed allowlists — fail-closed broken") } } diff --git a/jwksauth/claims.go b/jwksauth/claims.go index d70c38b..8e151dc 100644 --- a/jwksauth/claims.go +++ b/jwksauth/claims.go @@ -8,25 +8,25 @@ import ( "github.com/coreos/go-oidc/v3/oidc" ) -// Claims holds the non-standard JWT claims AuthGate emits in the payload. -// All fields are optional — services that don't use a given dimension can -// leave the corresponding [AccessRule] slice empty and the claim is ignored. +// Claims holds the AuthGate-specific JWT claims plus a generic Extras map +// for any caller-supplied keys the issuer included in the payload. // -// AuthGate's hierarchy is two-level: a Domain (e.g. "oa", "swrd", "hwrd") is -// the top-level partition, and an optional Tenant (e.g. "a76", "a78") names -// a sub-room inside a Domain. Tokens for Domains that have no sub-room -// concept simply omit the tenant claim. -// -// If a deployment uses namespaced claims (e.g. -// "https://authgate.example.com/domain"), copy this struct and adjust the -// json tags rather than monkey-patching the SDK. +// Domain, Project, and ServiceAccount are server-attested by AuthGate; +// configure the JWT payload prefix via [WithPrivateClaimPrefix]. Extras +// carries every other non-standard key — read individual values with +// [TokenInfo.Extra]. type Claims struct { - ClientID string `json:"client_id,omitempty"` - Scope string `json:"scope,omitempty"` - Domain string `json:"domain,omitempty"` - Tenant string `json:"tenant,omitempty"` - ServiceAccount string `json:"service_account,omitempty"` - Project string `json:"project,omitempty"` + ClientID string + Scope string + Domain string + ServiceAccount string + Project string + + // Extras carries any payload keys that are neither JWT/OIDC standard + // claims nor the three server-attested "_..." keys. Values are + // taken from the decoded JSON map by reference; callers should treat + // them as read-only. + Extras map[string]any } // TokenInfo is the result of a successful verification. It embeds the @@ -39,7 +39,7 @@ type Claims struct { type TokenInfo struct { *oidc.IDToken - // Claims is the decoded set of AuthGate custom claims. + // Claims is the decoded set of AuthGate custom claims plus Extras. Claims Claims // Scopes is the parsed scope list (strings.Fields(Claims.Scope)) cached @@ -58,24 +58,92 @@ func (t *TokenInfo) Domain() string { return strings.ToLower(t.Claims.Domain) } -// Tenant returns the case-folded tenant code (the optional sub-room inside a -// Domain). Returns "" when the token has no tenant claim — that is the -// documented "Domain has no sub-room" signal. Use t.Claims.Tenant if you -// need the original case. -func (t *TokenInfo) Tenant() string { - return strings.ToLower(t.Claims.Tenant) +// Extra returns the value associated with key in [Claims.Extras] (comma-ok +// style). Returns (nil, false) when the key is absent or [Claims.Extras] +// is nil. Use this to read caller-supplied claims that the SDK does not +// expose as a named field. +func (t *TokenInfo) Extra(key string) (any, bool) { + if t.Claims.Extras == nil { + return nil, false + } + v, ok := t.Claims.Extras[key] + return v, ok +} + +// staticReservedClaimKeys mirrors upstream AuthGate's registry of standard +// JWT/OIDC claim keys that are never surfaced via Claims.Extras. The three +// server-attested keys are excluded dynamically by newTokenInfo via the +// resolved [claimKeys]. +var staticReservedClaimKeys = map[string]struct{}{ + "iss": {}, "sub": {}, "aud": {}, "exp": {}, "nbf": {}, "iat": {}, "jti": {}, + "type": {}, "scope": {}, "user_id": {}, "client_id": {}, + "azp": {}, "amr": {}, "acr": {}, "auth_time": {}, "nonce": {}, "at_hash": {}, +} + +// claimKeys holds the resolved "_" payload keys for the +// three server-attested AuthGate claims. Construction-time once; read-only +// on the verify hot path. +type claimKeys struct { + domain string + project string + serviceAccount string +} + +// newClaimKeys composes the three server-attested payload keys from prefix. +// Mirrors upstream's EmittedName(prefix, logical) = prefix + "_" + logical. +func newClaimKeys(prefix string) claimKeys { + return claimKeys{ + domain: prefix + "_domain", + project: prefix + "_project", + serviceAccount: prefix + "_service_account", + } } -// newTokenInfo decodes the AuthGate-specific Claims from a verified IDToken -// and assembles the TokenInfo struct returned by both verifiers. -func newTokenInfo(tok *oidc.IDToken) (*TokenInfo, error) { - var extra Claims - if err := tok.Claims(&extra); err != nil { +// newTokenInfo decodes Claims and Extras from a verified IDToken using the +// resolved server-attested key names. +func newTokenInfo(tok *oidc.IDToken, keys claimKeys) (*TokenInfo, error) { + var raw map[string]any + if err := tok.Claims(&raw); err != nil { return nil, fmt.Errorf("decode JWT claims: %w", err) } + + c := Claims{ + ClientID: stringFromRaw(raw, "client_id"), + Scope: stringFromRaw(raw, "scope"), + Domain: stringFromRaw(raw, keys.domain), + Project: stringFromRaw(raw, keys.project), + ServiceAccount: stringFromRaw(raw, keys.serviceAccount), + } + + for k, v := range raw { + if _, reserved := staticReservedClaimKeys[k]; reserved { + continue + } + if k == keys.domain || k == keys.project || k == keys.serviceAccount { + continue + } + if c.Extras == nil { + c.Extras = make(map[string]any) + } + c.Extras[k] = v + } + return &TokenInfo{ IDToken: tok, - Claims: extra, - Scopes: strings.Fields(extra.Scope), + Claims: c, + Scopes: strings.Fields(c.Scope), }, nil } + +// stringFromRaw returns the string value of key in raw, or "" if absent or +// not a string. JWTs are string-only at the schema level for the keys we +// extract this way, so a non-string value is an anomalous token; the +// fail-closed AccessRule path handles missing values uniformly. +func stringFromRaw(raw map[string]any, key string) string { + if v, ok := raw[key]; ok { + if s, ok := v.(string); ok { + return s + } + } + return "" +} diff --git a/jwksauth/claims_prefix_test.go b/jwksauth/claims_prefix_test.go new file mode 100644 index 0000000..13b95ee --- /dev/null +++ b/jwksauth/claims_prefix_test.go @@ -0,0 +1,271 @@ +package jwksauth + +import ( + "context" + "net/http" + "strings" + "testing" + "time" +) + +// Happy path with the default prefix. Token carries +// extra_domain / extra_project / extra_service_account plus an arbitrary +// caller-supplied "tenant" key; AccessRule on all three server-attested +// dimensions hits, and the caller-supplied key surfaces via Extras. +func TestPrefixedClaims_DefaultPrefix_HappyPath(t *testing.T) { + fi := newFakeIssuer(t) + v, err := NewVerifier(t.Context(), fi.URL(), "api://x") + if err != nil { + t.Fatalf("NewVerifier: %v", err) + } + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{ + "extra_domain": "oa", + "extra_project": "p1", + "extra_service_account": "sync@oa", + "tenant": "a76", + }) + rule := AccessRule{ + Domains: []string{"oa"}, + Projects: []string{"p1"}, + ServiceAccounts: []string{"sync@oa"}, + } + rec := runMiddleware(t, v, rule, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+tok) + }) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + + info, err := v.Verify(context.Background(), tok) + if err != nil { + t.Fatalf("Verify: %v", err) + } + if info.Domain() != "oa" { + t.Errorf("Domain() = %q, want oa", info.Domain()) + } + if info.Claims.Project != "p1" { + t.Errorf("Project = %q, want p1", info.Claims.Project) + } + if info.Claims.ServiceAccount != "sync@oa" { + t.Errorf("ServiceAccount = %q, want sync@oa", info.Claims.ServiceAccount) + } + got, ok := info.Extra("tenant") + if !ok { + t.Fatalf("Extra(\"tenant\") missing") + } + if s, _ := got.(string); s != "a76" { + t.Errorf("Extra(\"tenant\") = %v, want a76", got) + } +} + +// With WithPrivateClaimPrefix("acme") the SDK reads "acme_domain" only; +// hard cutover means a token signed with "extra_domain" against an +// "acme"-configured verifier yields empty Domain and is rejected by +// AccessRule (no fallback to bare or default keys). +func TestPrefixedClaims_CustomPrefix(t *testing.T) { + fi := newFakeIssuer(t) + v, err := NewVerifier(t.Context(), fi.URL(), "api://x", WithPrivateClaimPrefix("acme")) + if err != nil { + t.Fatalf("NewVerifier: %v", err) + } + + t.Run("acme_prefix_hits", func(t *testing.T) { + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"acme_domain": "oa"}) + rec := runMiddleware(t, v, AccessRule{Domains: []string{"oa"}}, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+tok) + }) + if rec.Code != http.StatusOK { + t.Errorf("status = %d, want 200", rec.Code) + } + info, err := v.Verify(context.Background(), tok) + if err != nil { + t.Fatalf("Verify: %v", err) + } + if info.Claims.Domain != "oa" { + t.Errorf("Claims.Domain = %q, want oa", info.Claims.Domain) + } + }) + + t.Run("default_prefix_no_fallback", func(t *testing.T) { + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "oa"}) + info, err := v.Verify(context.Background(), tok) + if err != nil { + t.Fatalf("Verify: %v", err) + } + if info.Claims.Domain != "" { + t.Errorf( + "Claims.Domain = %q, want empty (wrong prefix must not fall back)", + info.Claims.Domain, + ) + } + got, ok := info.Extra("extra_domain") + if !ok { + t.Fatalf("extra_domain should land in Extras when prefix is acme") + } + if s, _ := got.(string); s != "oa" { + t.Errorf("Extras[extra_domain] = %v, want \"oa\"", got) + } + + rec := runMiddleware(t, v, AccessRule{Domains: []string{"oa"}}, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+tok) + }) + if rec.Code != http.StatusUnauthorized { + t.Errorf("status = %d, want 401 (Domain decoded as empty under wrong prefix)", rec.Code) + } + }) +} + +// A token carrying only the bare "domain" key (no extra_domain) must be +// rejected by an AccessRule on Domains. The bare key is not lost — it +// surfaces via Extras["domain"] — but it is not treated as server-attested. +func TestPrefixedClaims_BareDomainIgnored(t *testing.T) { + fi := newFakeIssuer(t) + v, err := NewVerifier(t.Context(), fi.URL(), "api://x") + if err != nil { + t.Fatalf("NewVerifier: %v", err) + } + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"domain": "oa"}) + + info, err := v.Verify(context.Background(), tok) + if err != nil { + t.Fatalf("Verify: %v", err) + } + if info.Claims.Domain != "" { + t.Errorf( + "Claims.Domain = %q, want empty (bare key must not be read as server-attested)", + info.Claims.Domain, + ) + } + got, ok := info.Extra("domain") + if !ok { + t.Fatalf("Extra(\"domain\") missing — bare key must still surface via Extras") + } + if s, _ := got.(string); s != "oa" { + t.Errorf("Extras[domain] = %v, want \"oa\"", got) + } + + rec := runMiddleware(t, v, AccessRule{Domains: []string{"oa"}}, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+tok) + }) + if rec.Code != http.StatusUnauthorized { + t.Errorf( + "status = %d, want 401 (bare \"domain\" must not satisfy Domain allowlist)", + rec.Code, + ) + } +} + +// Caller-supplied keys outside the server-attested registry never leak +// into the typed Claims fields, regardless of whether they collide with +// the prefix space. Both extra_foo and foo land in Extras under their +// own keys. +func TestPrefixedClaims_CallerSuppliedKeysDoNotPromote(t *testing.T) { + fi := newFakeIssuer(t) + v, err := NewVerifier(t.Context(), fi.URL(), "api://x") + if err != nil { + t.Fatalf("NewVerifier: %v", err) + } + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{ + "extra_foo": "bar", + "foo": "baz", + }) + + info, err := v.Verify(context.Background(), tok) + if err != nil { + t.Fatalf("Verify: %v", err) + } + got, ok := info.Extra("foo") + if !ok || got.(string) != "baz" { + t.Errorf("Extra(\"foo\") = (%v, %v), want (\"baz\", true)", got, ok) + } + got, ok = info.Extra("extra_foo") + if !ok || got.(string) != "bar" { + t.Errorf("Extra(\"extra_foo\") = (%v, %v), want (\"bar\", true)", got, ok) + } + if info.Claims.Domain != "" || info.Claims.Project != "" || info.Claims.ServiceAccount != "" { + t.Errorf( + "server-attested fields populated by caller-supplied keys: Domain=%q Project=%q ServiceAccount=%q", + info.Claims.Domain, + info.Claims.Project, + info.Claims.ServiceAccount, + ) + } +} + +// validatePrivateClaimPrefix table-driven positive and negative cases. +// The negatives mirror upstream validateJWTPrivateClaimPrefix rules: +// start with letter, [a-zA-Z0-9_]*, length 1-15, not ending in _. +func TestValidatePrivateClaimPrefix(t *testing.T) { + t.Run("valid", func(t *testing.T) { + cases := []string{ + "extra", + "acme", + "a", + "a_b", + "X9", + strings.Repeat("a", 15), // exactly the upper bound + } + for _, p := range cases { + t.Run(p, func(t *testing.T) { + if err := validatePrivateClaimPrefix(p); err != nil { + t.Errorf("validatePrivateClaimPrefix(%q) = %v, want nil", p, err) + } + }) + } + }) + + t.Run("invalid", func(t *testing.T) { + cases := []struct { + name string + in string + }{ + {"empty", ""}, + {"digit_first", "1abc"}, + {"hyphen", "a-b"}, + {"trailing_underscore", "abc_"}, + {"too_long", strings.Repeat("a", 16)}, + {"contains_space", "a b"}, + {"contains_chinese", "中文"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if err := validatePrivateClaimPrefix(tc.in); err == nil { + t.Errorf("validatePrivateClaimPrefix(%q) = nil, want error", tc.in) + } + }) + } + }) +} + +// NewVerifier and NewMultiVerifier propagate prefix validation as errors +// at construction time rather than panicking or silently using a bad +// prefix. +func TestNewVerifier_RejectsInvalidPrefix(t *testing.T) { + fi := newFakeIssuer(t) + + t.Run("Verifier", func(t *testing.T) { + _, err := NewVerifier( + t.Context(), fi.URL(), "api://x", + WithPrivateClaimPrefix("a-b"), + ) + if err == nil { + t.Fatal("expected error for invalid prefix") + } + if !strings.Contains(err.Error(), "private claim prefix") { + t.Errorf("error = %v, want mention of \"private claim prefix\"", err) + } + }) + + t.Run("MultiVerifier", func(t *testing.T) { + _, err := NewMultiVerifier( + t.Context(), []string{fi.URL()}, "api://x", + WithPrivateClaimPrefix("a-b"), + ) + if err == nil { + t.Fatal("expected error for invalid prefix") + } + if !strings.Contains(err.Error(), "private claim prefix") { + t.Errorf("error = %v, want mention of \"private claim prefix\"", err) + } + }) +} diff --git a/jwksauth/doc.go b/jwksauth/doc.go index b0c419c..2dfafab 100644 --- a/jwksauth/doc.go +++ b/jwksauth/doc.go @@ -32,13 +32,21 @@ // mux.Handle("/api/profile", jwksauth.Middleware(v, jwksauth.AccessRule{})(profileHandler)) // mux.Handle("/api/data", jwksauth.Middleware(v, jwksauth.AccessRule{Scopes: []string{"email"}})(dataHandler)) // -// # Multiple issuers (multi-region / multi-domain / migration) +// # Server-attested private claims and the prefix +// +// AuthGate emits three private claims — Domain, Project, and ServiceAccount +// — under a configurable prefix (default "extra"), so the JWT payload keys +// are "extra_domain", "extra_project", and "extra_service_account". The +// SDK reads them out of the box; if your AuthGate deployment has overridden +// JWT_PRIVATE_CLAIM_PREFIX, pass the same value via +// [WithPrivateClaimPrefix]. // -// AuthGate's hierarchy is two-level: a Domain (e.g. "oa", "swrd", "hwrd") is -// the top-level partition, and an optional Tenant (e.g. "a76", "a78") names -// a sub-room inside a Domain. Tokens carry domain and tenant as two -// independent claims; tokens for Domains that have no sub-room concept omit -// the tenant claim entirely. +// Any other non-standard payload keys (for example a caller-supplied +// "tenant") are surfaced via [Claims.Extras]; read them with +// [TokenInfo.Extra]. Caller-supplied keys are not part of [AccessRule] or +// the cross-issuer Domain pinning. +// +// # Multiple issuers (multi-region / multi-domain / migration) // // For a service that accepts tokens from several AuthGates: // @@ -47,8 +55,8 @@ // "https://api.example.com") // if err != nil { log.Fatal(err) } // // Optional cross-domain defense — strongly recommended with short -// // domain codes. Tenants live entirely inside a Domain and are not part -// // of cross-issuer pinning. +// // domain codes. Only Domain participates; caller-supplied claims do +// // not enter the cross-issuer pinning. // if err := mv.SetIssuerDomains("https://auth-a.example.com=oa,hwrd;https://auth-b.example.com=swrd"); err != nil { // log.Fatal(err) // } diff --git a/jwksauth/middleware_test.go b/jwksauth/middleware_test.go index 3f093e5..f9d0ad3 100644 --- a/jwksauth/middleware_test.go +++ b/jwksauth/middleware_test.go @@ -114,12 +114,12 @@ func TestVerifier_HappyPath(t *testing.T) { t.Fatalf("NewVerifier: %v", err) } tok := fi.Sign(t, "api://example", 5*time.Minute, map[string]any{ - "client_id": "cli", - "scope": "email profile", - "domain": "OA", - "tenant": "A76", - "service_account": "sync@oa", - "project": "p1", + "client_id": "cli", + "scope": "email profile", + "extra_domain": "OA", + "tenant": "A76", + "extra_service_account": "sync@oa", + "extra_project": "p1", }) info, err := v.Verify(context.Background(), tok) if err != nil { @@ -134,8 +134,8 @@ func TestVerifier_HappyPath(t *testing.T) { if info.Domain() != "oa" { t.Errorf("Domain() = %q, want lower-cased 'oa'", info.Domain()) } - if info.Tenant() != "a76" { - t.Errorf("Tenant() = %q, want lower-cased 'a76'", info.Tenant()) + if got, ok := info.Extra("tenant"); !ok || got.(string) != "A76" { + t.Errorf("Extra(\"tenant\") = (%v, %v), want (\"A76\", true)", got, ok) } } @@ -306,7 +306,7 @@ func TestMiddleware_DomainAllowlist(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"domain": tc.domain}) + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": tc.domain}) rec := runMiddleware(t, v, AccessRule{Domains: tc.rule}, func(req *http.Request) { req.Header.Set("Authorization", "Bearer "+tok) }) @@ -324,9 +324,9 @@ func TestMiddleware_HappyPath_InjectsContext(t *testing.T) { t.Fatalf("NewVerifier: %v", err) } tok := fi.Sign(t, "api://x", time.Minute, map[string]any{ - "scope": "email", - "domain": "OA", - "tenant": "A76", + "scope": "email", + "extra_domain": "OA", + "tenant": "A76", }) called := false handler := Middleware(v, AccessRule{ @@ -341,15 +341,12 @@ func TestMiddleware_HappyPath_InjectsContext(t *testing.T) { if info.Domain() != "oa" { t.Errorf("Domain = %q, want oa", info.Domain()) } - if info.Tenant() != "a76" { - t.Errorf("Tenant = %q, want a76", info.Tenant()) + if got, ok := info.Extra("tenant"); !ok || got.(string) != "A76" { + t.Errorf("Extra(\"tenant\") = (%v, %v), want (\"A76\", true)", got, ok) } if info.Claims.Domain != "OA" { t.Errorf("Claims.Domain = %q, want OA", info.Claims.Domain) } - if info.Claims.Tenant != "A76" { - t.Errorf("Claims.Tenant = %q, want A76", info.Claims.Tenant) - } called = true })) req := httptest.NewRequest(http.MethodGet, "/", nil) @@ -365,16 +362,17 @@ func TestMiddleware_HappyPath_InjectsContext(t *testing.T) { } } -// TestMiddleware_DomainPresent_TenantAbsent pins the contract that a token -// carrying a Domain but no Tenant claim is accepted when the Domain is in -// the allowlist; the handler observes Tenant() == "". -func TestMiddleware_DomainPresent_TenantAbsent(t *testing.T) { +// TestMiddleware_DomainPresent_NoExtras pins the contract that a token +// carrying only the server-attested Domain (and no caller-supplied keys) +// is accepted when the Domain is in the allowlist; the handler observes +// an empty Extras map. +func TestMiddleware_DomainPresent_NoExtras(t *testing.T) { fi := newFakeIssuer(t) v, err := NewVerifier(t.Context(), fi.URL(), "api://x") if err != nil { t.Fatalf("NewVerifier: %v", err) } - tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"domain": "oa"}) + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "oa"}) called := false handler := Middleware(v, AccessRule{ Domains: []string{"oa"}, @@ -384,11 +382,8 @@ func TestMiddleware_DomainPresent_TenantAbsent(t *testing.T) { t.Error("TokenInfoFromContext returned !ok") return } - if info.Claims.Tenant != "" { - t.Errorf("Claims.Tenant = %q, want empty (no sub-room)", info.Claims.Tenant) - } - if info.Tenant() != "" { - t.Errorf("Tenant() = %q, want empty", info.Tenant()) + if _, ok := info.Extra("tenant"); ok { + t.Errorf("Extra(\"tenant\") = ok, want absent") } called = true })) @@ -407,13 +402,10 @@ func TestMiddleware_DomainPresent_TenantAbsent(t *testing.T) { // TestMiddleware_AcceptsTokenWithNoCustomClaims pins the contract that // the SDK verifies any AuthGate-issued token regardless of which custom -// claims it carries. AuthGate today emits the standard claims (iss, sub, -// aud, exp, nbf, scope) plus optional service_account and project; the -// Domain and Tenant fields on Claims are forward-looking and remain -// empty until the server starts populating them. With AccessRule{} (no -// allowlists) and no SetIssuerDomains enforcement, only signature, iss, -// aud, exp, and nbf are checked — guaranteeing the SDK works regardless -// of whether the server emits any custom claims at all. +// claims it carries. With AccessRule{} (no allowlists) and no +// SetIssuerDomains enforcement, only signature, iss, aud, exp, and nbf +// are checked — guaranteeing the SDK works regardless of whether the +// server emits any private claims at all. func TestMiddleware_AcceptsTokenWithNoCustomClaims(t *testing.T) { fi := newFakeIssuer(t) v, err := NewVerifier(t.Context(), fi.URL(), "api://x") @@ -437,7 +429,7 @@ func TestMultiVerifier_RoutesByIssuer(t *testing.T) { t.Fatalf("NewMultiVerifier: %v", err) } - tokA := a.Sign(t, "api://x", time.Minute, map[string]any{"domain": "oa"}) + tokA := a.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "oa"}) infoA, err := mv.Verify(context.Background(), tokA) if err != nil { t.Fatalf("Verify A: %v", err) @@ -446,7 +438,7 @@ func TestMultiVerifier_RoutesByIssuer(t *testing.T) { t.Errorf("issuer = %q, want %q", infoA.Issuer, a.URL()) } - tokB := b.Sign(t, "api://x", time.Minute, map[string]any{"domain": "swrd"}) + tokB := b.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "swrd"}) infoB, err := mv.Verify(context.Background(), tokB) if err != nil { t.Fatalf("Verify B: %v", err) @@ -484,7 +476,7 @@ func TestMultiVerifier_CrossDomainDefense(t *testing.T) { // Issuer A claims domain 'swrd' (which belongs to B) → reject. The // error must not echo back the configured allowlist for this issuer. - tok := a.Sign(t, "api://x", time.Minute, map[string]any{"domain": "swrd"}) + tok := a.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "swrd"}) _, rejErr := mv.Verify(context.Background(), tok) if rejErr == nil { t.Fatal("expected cross-domain rejection") @@ -494,7 +486,7 @@ func TestMultiVerifier_CrossDomainDefense(t *testing.T) { } // Same issuer, its own domain → accept. - tok = a.Sign(t, "api://x", time.Minute, map[string]any{"domain": "oa"}) + tok = a.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "oa"}) if _, err := mv.Verify(context.Background(), tok); err != nil { t.Fatalf("legit token rejected: %v", err) } @@ -510,7 +502,7 @@ func TestMultiVerifier_SetIssuerDomainsRace(t *testing.T) { if err != nil { t.Fatalf("NewMultiVerifier: %v", err) } - tok := a.Sign(t, "api://x", time.Minute, map[string]any{"domain": "oa"}) + tok := a.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "oa"}) stop := make(chan struct{}) done := make(chan struct{}, 2) @@ -694,7 +686,7 @@ func TestMiddleware_LogsPolicyReject(t *testing.T) { if err != nil { t.Fatalf("NewVerifier: %v", err) } - tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"domain": "swrd"}) + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "swrd"}) cl := &captureLogger{} rec := runMiddleware(t, v, AccessRule{Domains: []string{"oa"}}, func(req *http.Request) { diff --git a/jwksauth/multi_verifier.go b/jwksauth/multi_verifier.go index 6c6690a..10ae6f5 100644 --- a/jwksauth/multi_verifier.go +++ b/jwksauth/multi_verifier.go @@ -43,6 +43,10 @@ type MultiVerifier struct { issuerDomains atomic.Pointer[map[string][]string] timeout time.Duration + + // keys are the resolved server-attested payload keys; see + // [WithPrivateClaimPrefix]. + keys claimKeys } // NewMultiVerifier builds a multi-issuer verifier. Every issuer in issuers @@ -99,6 +103,9 @@ func newMultiVerifier( o.apply(&cfg) } } + if err := validatePrivateClaimPrefix(cfg.privateClaimPrefix); err != nil { + return nil, fmt.Errorf("jwksauth: invalid private claim prefix: %w", err) + } discoverCtx, cancel := context.WithTimeout(ctx, cfg.discoveryTimeout) defer cancel() @@ -110,6 +117,7 @@ func newMultiVerifier( return &MultiVerifier{ verifiers: verifiers, timeout: cfg.verifyTimeout, + keys: newClaimKeys(cfg.privateClaimPrefix), }, nil } @@ -130,9 +138,9 @@ func newMultiVerifier( // enforced strictly so a typo or operational mistake fails fast at // configuration time rather than silently disabling the check. // -// Cross-issuer enforcement is Domain-level only. The optional sub-room -// Tenant claim lives entirely inside a Domain, so there is no cross-issuer -// Tenant exposure to defend against. +// Cross-issuer enforcement is Domain-level only. Caller-supplied claims +// surfaced via [Claims.Extras] are not part of cross-domain enforcement — +// only the server-attested Domain participates here. // // Pass an empty string to disable cross-domain enforcement; safe to call // concurrently with [MultiVerifier.Verify] (the swap is atomic). @@ -210,7 +218,7 @@ func (v *MultiVerifier) Verify(ctx context.Context, raw string) (*TokenInfo, err if err != nil { return nil, err } - info, err := newTokenInfo(tok) + info, err := newTokenInfo(tok, v.keys) if err != nil { return nil, err } diff --git a/jwksauth/options.go b/jwksauth/options.go index 05a78d0..0c68925 100644 --- a/jwksauth/options.go +++ b/jwksauth/options.go @@ -1,6 +1,27 @@ package jwksauth -import "time" +import ( + "errors" + "fmt" + "regexp" + "time" +) + +var errEmptyPrivateClaimPrefix = errors.New("must not be empty") + +// defaultPrivateClaimPrefix matches upstream AuthGate's JWT_PRIVATE_CLAIM_PREFIX +// default. Server and SDK must agree byte-for-byte; if a deployment has +// overridden this on the server, configure the SDK with [WithPrivateClaimPrefix]. +const defaultPrivateClaimPrefix = "extra" + +// maxPrivateClaimPrefixLen mirrors upstream's length cap on the prefix. +const maxPrivateClaimPrefixLen = 15 + +// privateClaimPrefixPattern mirrors upstream validateJWTPrivateClaimPrefix: +// must start with a letter; subsequent characters are letters, digits, or +// underscore. The "must not end with underscore" rule is enforced +// separately so the error message can name that specific violation. +var privateClaimPrefixPattern = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9_]*$`) // Option configures [Verifier] and [MultiVerifier] construction. type Option interface { @@ -11,14 +32,16 @@ type Option interface { // constructors. It is unexported on purpose; callers configure it via the // Option helpers. type verifierConfig struct { - verifyTimeout time.Duration - discoveryTimeout time.Duration + verifyTimeout time.Duration + discoveryTimeout time.Duration + privateClaimPrefix string } func defaultVerifierConfig() verifierConfig { return verifierConfig{ - verifyTimeout: 5 * time.Second, - discoveryTimeout: 15 * time.Second, + verifyTimeout: 5 * time.Second, + discoveryTimeout: 15 * time.Second, + privateClaimPrefix: defaultPrivateClaimPrefix, } } @@ -49,3 +72,49 @@ func WithDiscoveryTimeout(d time.Duration) Option { } }) } + +// WithPrivateClaimPrefix configures the prefix the SDK uses when reading +// AuthGate's server-attested private claims (Domain, Project, +// ServiceAccount). Defaults to "extra"; pass the value here only when +// the AuthGate deployment has overridden JWT_PRIVATE_CLAIM_PREFIX. Server +// and SDK must agree byte-for-byte — reading with the wrong prefix +// yields empty fields and (when AccessRule covers those dimensions) +// fails closed. +// +// An empty string is treated as "use the default" (consistent with +// [WithVerifyTimeout]'s zero-input handling). Format errors are returned +// from [NewVerifier] / [NewMultiVerifier], never silently ignored. +func WithPrivateClaimPrefix(p string) Option { + return optionFunc(func(c *verifierConfig) { + if p != "" { + c.privateClaimPrefix = p + } + }) +} + +// validatePrivateClaimPrefix mirrors upstream validateJWTPrivateClaimPrefix. +// Rules: 1-15 characters, starts with a letter, only letters/digits/underscore, +// must not end with an underscore (which would yield "__" +// after the EmittedName join). +// +// Reserved-key collision checks are intentionally left to the server side; +// duplicating them in the SDK adds maintenance with no defense gain. +func validatePrivateClaimPrefix(p string) error { + if p == "" { + return errEmptyPrivateClaimPrefix + } + if len(p) > maxPrivateClaimPrefixLen { + return fmt.Errorf("%q exceeds %d characters", p, maxPrivateClaimPrefixLen) + } + if !privateClaimPrefixPattern.MatchString(p) { + return fmt.Errorf( + "%q must match %s", + p, + privateClaimPrefixPattern.String(), + ) + } + if p[len(p)-1] == '_' { + return fmt.Errorf("%q must not end with underscore", p) + } + return nil +} diff --git a/jwksauth/verifier.go b/jwksauth/verifier.go index 1737b3e..ba6a8fe 100644 --- a/jwksauth/verifier.go +++ b/jwksauth/verifier.go @@ -37,6 +37,10 @@ type Verifier struct { canonicalIssuer string timeout time.Duration + + // keys are the resolved server-attested payload keys; see + // [WithPrivateClaimPrefix]. + keys claimKeys } // NewVerifier builds a single-issuer Verifier that requires the `aud` @@ -90,6 +94,9 @@ func newVerifier( o.apply(&cfg) } } + if err := validatePrivateClaimPrefix(cfg.privateClaimPrefix); err != nil { + return nil, fmt.Errorf("jwksauth: invalid private claim prefix: %w", err) + } discoverCtx, cancel := context.WithTimeout(ctx, cfg.discoveryTimeout) defer cancel() @@ -102,6 +109,7 @@ func newVerifier( verifier: oidcVerifier, canonicalIssuer: canonical, timeout: cfg.verifyTimeout, + keys: newClaimKeys(cfg.privateClaimPrefix), }, nil } @@ -155,5 +163,5 @@ func (v *Verifier) Verify(ctx context.Context, raw string) (*TokenInfo, error) { if err != nil { return nil, err } - return newTokenInfo(tok) + return newTokenInfo(tok, v.keys) } From 39d06f1ee5186c9736a9bba2f7d06945edc861e9 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 2 May 2026 13:50:19 +0800 Subject: [PATCH 2/7] test(jwksauth): guard Extra type assertions and clarify claim docs - Use guarded type assertions on TokenInfo.Extra results so non-string values produce a clean test failure instead of panicking - Reword the README to acknowledge that server-attested private claims are optional, not always emitted - Document on Claims that the struct is populated by the SDK verifier and is not intended for direct JSON marshal/unmarshal Co-Authored-By: Claude Opus 4.7 (1M context) --- jwksauth/README.md | 10 ++++++---- jwksauth/claims.go | 5 +++++ jwksauth/claims_prefix_test.go | 13 +++++++++---- jwksauth/middleware_test.go | 14 ++++++++++---- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/jwksauth/README.md b/jwksauth/README.md index fa30219..c94ca06 100644 --- a/jwksauth/README.md +++ b/jwksauth/README.md @@ -62,10 +62,12 @@ func profile(w http.ResponseWriter, r *http.Request) { ## Server-attested private claims and the prefix -AuthGate emits three private claims on every token: **Domain**, **Project**, -**ServiceAccount**. They appear in the payload under a configurable prefix -(default `extra`), so the JWT keys are `extra_domain`, `extra_project`, -`extra_service_account`. The SDK reads them out of the box. +AuthGate may emit up to three private claims on a token: **Domain**, +**Project**, **ServiceAccount**. Each is optional — tokens that don't +need a given dimension simply omit the claim. When present they appear +in the payload under a configurable prefix (default `extra`), so the +JWT keys are `extra_domain`, `extra_project`, `extra_service_account`. +The SDK reads them out of the box. ```json { diff --git a/jwksauth/claims.go b/jwksauth/claims.go index 8e151dc..6835a61 100644 --- a/jwksauth/claims.go +++ b/jwksauth/claims.go @@ -15,6 +15,11 @@ import ( // configure the JWT payload prefix via [WithPrivateClaimPrefix]. Extras // carries every other non-standard key — read individual values with // [TokenInfo.Extra]. +// +// Claims is populated by the SDK's verifier from a verified IDToken; it +// is not intended for direct JSON marshal/unmarshal by callers and +// therefore carries no json tags. If you need the raw payload, use the +// embedded [oidc.IDToken] on [TokenInfo]. type Claims struct { ClientID string Scope string diff --git a/jwksauth/claims_prefix_test.go b/jwksauth/claims_prefix_test.go index 13b95ee..87c0419 100644 --- a/jwksauth/claims_prefix_test.go +++ b/jwksauth/claims_prefix_test.go @@ -175,12 +175,17 @@ func TestPrefixedClaims_CallerSuppliedKeysDoNotPromote(t *testing.T) { t.Fatalf("Verify: %v", err) } got, ok := info.Extra("foo") - if !ok || got.(string) != "baz" { - t.Errorf("Extra(\"foo\") = (%v, %v), want (\"baz\", true)", got, ok) + if s, isStr := got.(string); !ok || !isStr || s != "baz" { + t.Errorf("Extra(\"foo\") = %v (%T), ok=%v; want \"baz\" (string), ok=true", got, got, ok) } got, ok = info.Extra("extra_foo") - if !ok || got.(string) != "bar" { - t.Errorf("Extra(\"extra_foo\") = (%v, %v), want (\"bar\", true)", got, ok) + if s, isStr := got.(string); !ok || !isStr || s != "bar" { + t.Errorf( + "Extra(\"extra_foo\") = %v (%T), ok=%v; want \"bar\" (string), ok=true", + got, + got, + ok, + ) } if info.Claims.Domain != "" || info.Claims.Project != "" || info.Claims.ServiceAccount != "" { t.Errorf( diff --git a/jwksauth/middleware_test.go b/jwksauth/middleware_test.go index f9d0ad3..74c5dac 100644 --- a/jwksauth/middleware_test.go +++ b/jwksauth/middleware_test.go @@ -134,8 +134,11 @@ func TestVerifier_HappyPath(t *testing.T) { if info.Domain() != "oa" { t.Errorf("Domain() = %q, want lower-cased 'oa'", info.Domain()) } - if got, ok := info.Extra("tenant"); !ok || got.(string) != "A76" { - t.Errorf("Extra(\"tenant\") = (%v, %v), want (\"A76\", true)", got, ok) + got, ok := info.Extra("tenant") + if !ok { + t.Errorf("Extra(\"tenant\") missing, want \"A76\"") + } else if s, isStr := got.(string); !isStr || s != "A76" { + t.Errorf("Extra(\"tenant\") = %v (%T), want \"A76\" (string)", got, got) } } @@ -341,8 +344,11 @@ func TestMiddleware_HappyPath_InjectsContext(t *testing.T) { if info.Domain() != "oa" { t.Errorf("Domain = %q, want oa", info.Domain()) } - if got, ok := info.Extra("tenant"); !ok || got.(string) != "A76" { - t.Errorf("Extra(\"tenant\") = (%v, %v), want (\"A76\", true)", got, ok) + got, ok := info.Extra("tenant") + if !ok { + t.Errorf("Extra(\"tenant\") missing, want \"A76\"") + } else if s, isStr := got.(string); !isStr || s != "A76" { + t.Errorf("Extra(\"tenant\") = %v (%T), want \"A76\" (string)", got, got) } if info.Claims.Domain != "OA" { t.Errorf("Claims.Domain = %q, want OA", info.Claims.Domain) From 3ffe1345226fdb5ceca09208218983490ae23730 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 2 May 2026 13:59:23 +0800 Subject: [PATCH 3/7] docs(jwksauth): clarify reserved-key set vs Extras coverage - Reword the staticReservedClaimKeys comment to describe what the set actually represents (a hand-picked exclusion list mixing JWT, OIDC, and AuthGate-specific keys), not the full OIDC standard registry - Update the README's caller-supplied-keys paragraph to note that OIDC claims the SDK does not name explicitly (email, name, etc.) will land in Extras when the issuer emits them Co-Authored-By: Claude Opus 4.7 (1M context) --- jwksauth/README.md | 12 ++++++++---- jwksauth/claims.go | 16 ++++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/jwksauth/README.md b/jwksauth/README.md index c94ca06..32c9b82 100644 --- a/jwksauth/README.md +++ b/jwksauth/README.md @@ -162,10 +162,14 @@ prefix (`extra`); under a custom prefix the keys become | `ServiceAccounts` | `extra_service_account` | exact | Case-sensitive | | `Projects` | `extra_project` | exact | Case-sensitive | -Caller-supplied keys (anything else in the JWT payload that isn't a JWT/OIDC -standard claim) are not part of the allowlist surface. Read them from -`Claims.Extras` via `TokenInfo.Extra(key)` and apply your own logic in the -handler when needed. +Caller-supplied keys (any payload key not in the SDK's reserved-key set +and not one of the three server-attested `_domain` / +`_project` / `_service_account` keys) are not part of the +allowlist surface. They surface on `Claims.Extras`; read individual values +with `TokenInfo.Extra(key)` and apply your own logic in the handler when +needed. Note that OIDC standard claims the SDK does not name explicitly +(for example `email`, `name`) will also land in Extras if the issuer +emits them. Allowlist mismatches return `401 invalid_token` (generic) so the allowlist itself is not probeable. The full reason is logged server-side via the diff --git a/jwksauth/claims.go b/jwksauth/claims.go index 6835a61..02b00a1 100644 --- a/jwksauth/claims.go +++ b/jwksauth/claims.go @@ -75,10 +75,18 @@ func (t *TokenInfo) Extra(key string) (any, bool) { return v, ok } -// staticReservedClaimKeys mirrors upstream AuthGate's registry of standard -// JWT/OIDC claim keys that are never surfaced via Claims.Extras. The three -// server-attested keys are excluded dynamically by newTokenInfo via the -// resolved [claimKeys]. +// staticReservedClaimKeys is the set of payload keys this SDK +// intentionally excludes from Claims.Extras. It mirrors upstream +// AuthGate's reserved set, which mixes RFC 7519 standard JWT keys +// (iss, sub, aud, exp, nbf, iat, jti), a subset of OIDC keys (azp, amr, +// acr, auth_time, nonce, at_hash), and AuthGate-specific keys exposed +// as named [Claims] fields (scope, client_id) plus a few other +// AuthGate-emitted keys (type, user_id). It is NOT the full IANA OIDC +// claim registry — common OIDC claims the SDK does not name explicitly +// (e.g. email, name) will surface via Extras when the issuer emits them. +// +// The three server-attested keys are excluded dynamically by +// newTokenInfo via the resolved [claimKeys]. var staticReservedClaimKeys = map[string]struct{}{ "iss": {}, "sub": {}, "aud": {}, "exp": {}, "nbf": {}, "iat": {}, "jti": {}, "type": {}, "scope": {}, "user_id": {}, "client_id": {}, From 1ad513410f7c954be807ab9ee5438a8a96329a86 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 2 May 2026 14:05:10 +0800 Subject: [PATCH 4/7] refactor(jwksauth): trim whitespace in WithPrivateClaimPrefix - Apply strings.TrimSpace inside WithPrivateClaimPrefix so values sourced from environment variables or config files don't trip prefix validation on incidental leading/trailing whitespace - Treat whitespace-only input the same as empty (use the default prefix), consistent with the existing zero-input convention - Add a focused test pinning both the trimmed-prefix decode path and the whitespace-only fallback to default Co-Authored-By: Claude Opus 4.7 (1M context) --- jwksauth/claims_prefix_test.go | 49 ++++++++++++++++++++++++++++++++++ jwksauth/options.go | 12 +++++---- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/jwksauth/claims_prefix_test.go b/jwksauth/claims_prefix_test.go index 87c0419..2d48e2f 100644 --- a/jwksauth/claims_prefix_test.go +++ b/jwksauth/claims_prefix_test.go @@ -274,3 +274,52 @@ func TestNewVerifier_RejectsInvalidPrefix(t *testing.T) { } }) } + +// WithPrivateClaimPrefix trims surrounding whitespace before validation so +// values sourced from env/config don't trip the format check, and treats +// whitespace-only input the same as empty (use default). +func TestWithPrivateClaimPrefix_TrimsWhitespace(t *testing.T) { + fi := newFakeIssuer(t) + + t.Run("trimmed_prefix_decodes", func(t *testing.T) { + v, err := NewVerifier( + t.Context(), fi.URL(), "api://x", + WithPrivateClaimPrefix(" acme "), + ) + if err != nil { + t.Fatalf("NewVerifier: %v", err) + } + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"acme_domain": "oa"}) + info, err := v.Verify(t.Context(), tok) + if err != nil { + t.Fatalf("Verify: %v", err) + } + if info.Claims.Domain != "oa" { + t.Errorf( + "Claims.Domain = %q, want oa (trimmed prefix should resolve to acme)", + info.Claims.Domain, + ) + } + }) + + t.Run("whitespace_only_uses_default", func(t *testing.T) { + v, err := NewVerifier( + t.Context(), fi.URL(), "api://x", + WithPrivateClaimPrefix(" "), + ) + if err != nil { + t.Fatalf("NewVerifier: %v", err) + } + tok := fi.Sign(t, "api://x", time.Minute, map[string]any{"extra_domain": "oa"}) + info, err := v.Verify(t.Context(), tok) + if err != nil { + t.Fatalf("Verify: %v", err) + } + if info.Claims.Domain != "oa" { + t.Errorf( + "Claims.Domain = %q, want oa (whitespace-only should fall back to default \"extra\")", + info.Claims.Domain, + ) + } + }) +} diff --git a/jwksauth/options.go b/jwksauth/options.go index 0c68925..13ed86a 100644 --- a/jwksauth/options.go +++ b/jwksauth/options.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "regexp" + "strings" "time" ) @@ -81,13 +82,14 @@ func WithDiscoveryTimeout(d time.Duration) Option { // yields empty fields and (when AccessRule covers those dimensions) // fails closed. // -// An empty string is treated as "use the default" (consistent with -// [WithVerifyTimeout]'s zero-input handling). Format errors are returned -// from [NewVerifier] / [NewMultiVerifier], never silently ignored. +// Surrounding whitespace is trimmed; an empty or whitespace-only string is +// treated as "use the default" (consistent with [WithVerifyTimeout]'s +// zero-input handling). Format errors are returned from [NewVerifier] / +// [NewMultiVerifier], never silently ignored. func WithPrivateClaimPrefix(p string) Option { return optionFunc(func(c *verifierConfig) { - if p != "" { - c.privateClaimPrefix = p + if trimmed := strings.TrimSpace(p); trimmed != "" { + c.privateClaimPrefix = trimmed } }) } From 5a6d0bed9fda3704d4e2f005fb341c08ed1143a1 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 2 May 2026 14:13:58 +0800 Subject: [PATCH 5/7] docs(jwksauth): tighten Claims JSON-marshal note and fix duplicated With - Reword the Claims godoc to be precise about Go's default JSON marshal behavior on a tag-less struct (exported fields are emitted under their Go names like ClientID, not the snake_case JWT keys), instead of suggesting JSON marshal is somehow blocked - Drop the duplicated "With" in the TestPrefixedClaims_CustomPrefix doc comment Co-Authored-By: Claude Opus 4.7 (1M context) --- jwksauth/claims.go | 12 ++++++++---- jwksauth/claims_prefix_test.go | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/jwksauth/claims.go b/jwksauth/claims.go index 02b00a1..347f43f 100644 --- a/jwksauth/claims.go +++ b/jwksauth/claims.go @@ -16,10 +16,14 @@ import ( // carries every other non-standard key — read individual values with // [TokenInfo.Extra]. // -// Claims is populated by the SDK's verifier from a verified IDToken; it -// is not intended for direct JSON marshal/unmarshal by callers and -// therefore carries no json tags. If you need the raw payload, use the -// embedded [oidc.IDToken] on [TokenInfo]. +// Claims is populated by the SDK's verifier from a verified IDToken via +// custom decoding (not via struct-tag JSON unmarshal), so it deliberately +// carries no json tags — that keeps the struct from advertising payload +// keys that no longer match the prefixed wire format. As a side effect, +// passing a [Claims] value to encoding/json will marshal exported fields +// under their Go names ("ClientID", "Domain", ...) rather than the +// snake_case JWT keys; if you need the raw payload, read it from the +// embedded [oidc.IDToken] on [TokenInfo] instead. type Claims struct { ClientID string Scope string diff --git a/jwksauth/claims_prefix_test.go b/jwksauth/claims_prefix_test.go index 2d48e2f..873504b 100644 --- a/jwksauth/claims_prefix_test.go +++ b/jwksauth/claims_prefix_test.go @@ -58,7 +58,7 @@ func TestPrefixedClaims_DefaultPrefix_HappyPath(t *testing.T) { } } -// With WithPrivateClaimPrefix("acme") the SDK reads "acme_domain" only; +// WithPrivateClaimPrefix("acme") makes the SDK read "acme_domain" only; // hard cutover means a token signed with "extra_domain" against an // "acme"-configured verifier yields empty Domain and is rejected by // AccessRule (no fallback to bare or default keys). From 75d8e443e90a080e67415f911081dcaee7b9e299 Mon Sep 17 00:00:00 2001 From: appleboy Date: Sun, 3 May 2026 07:31:50 +0800 Subject: [PATCH 6/7] docs(jwksauth): align Extras field doc with reserved-key reality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extras' field comment claimed it carried "any payload keys that are neither JWT/OIDC standard claims" — but the SDK only filters a hand-picked reserved set, so common OIDC claims like email and name still surface via Extras. Reword the comment to reference the actual reserved set, and update TestMiddleware_DomainPresent_NoExtras's docstring to acknowledge that newTokenInfo leaves Claims.Extras nil (rather than allocating an empty map) when no caller-supplied keys exist; assert that nil contract explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) --- jwksauth/claims.go | 8 ++++++-- jwksauth/middleware_test.go | 6 +++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/jwksauth/claims.go b/jwksauth/claims.go index 347f43f..63d3e8a 100644 --- a/jwksauth/claims.go +++ b/jwksauth/claims.go @@ -31,8 +31,12 @@ type Claims struct { ServiceAccount string Project string - // Extras carries any payload keys that are neither JWT/OIDC standard - // claims nor the three server-attested "_..." keys. Values are + // Extras carries any payload keys that are neither in the SDK's + // reserved-key set (see [staticReservedClaimKeys]) nor the three + // server-attested "_..." keys. The reserved set covers RFC + // 7519 standard JWT keys and a hand-picked subset of OIDC keys, so + // common OIDC keys the SDK does not name explicitly (e.g. email, + // name) will surface here when the issuer emits them. Values are // taken from the decoded JSON map by reference; callers should treat // them as read-only. Extras map[string]any diff --git a/jwksauth/middleware_test.go b/jwksauth/middleware_test.go index 74c5dac..7e6f73a 100644 --- a/jwksauth/middleware_test.go +++ b/jwksauth/middleware_test.go @@ -371,7 +371,8 @@ func TestMiddleware_HappyPath_InjectsContext(t *testing.T) { // TestMiddleware_DomainPresent_NoExtras pins the contract that a token // carrying only the server-attested Domain (and no caller-supplied keys) // is accepted when the Domain is in the allowlist; the handler observes -// an empty Extras map. +// a nil Claims.Extras map (newTokenInfo only allocates the map when at +// least one non-reserved key is present). func TestMiddleware_DomainPresent_NoExtras(t *testing.T) { fi := newFakeIssuer(t) v, err := NewVerifier(t.Context(), fi.URL(), "api://x") @@ -391,6 +392,9 @@ func TestMiddleware_DomainPresent_NoExtras(t *testing.T) { if _, ok := info.Extra("tenant"); ok { t.Errorf("Extra(\"tenant\") = ok, want absent") } + if info.Claims.Extras != nil { + t.Errorf("Claims.Extras = %v, want nil", info.Claims.Extras) + } called = true })) req := httptest.NewRequest(http.MethodGet, "/", nil) From 34ab528adcef5d4c1f4fd1775b398526226ae799 Mon Sep 17 00:00:00 2001 From: appleboy Date: Sun, 3 May 2026 07:44:43 +0800 Subject: [PATCH 7/7] docs(jwksauth): document WithPrivateClaimPrefix multi-issuer and empty-input semantics Two doc fixes flagged by review on the option godoc: 1. Multi-issuer caveat. MultiVerifier caches a single resolved claimKeys set, so passing issuers with different JWT_PRIVATE_CLAIM_PREFIX values to one MultiVerifier silently misreads claims for the mismatched issuer(s). Document that one MultiVerifier serves a single prefix; users with mixed prefixes should build one Verifier per prefix. 2. Empty-input behavior. The previous wording ("treated as use the default") suggested WithPrivateClaimPrefix("") would revert an earlier non-default call, but the implementation simply skips the assignment when trimmed input is empty. Reword to "no-op: leaves the previously-configured prefix in place" so the doc and code agree. Co-Authored-By: Claude Opus 4.7 (1M context) --- jwksauth/options.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/jwksauth/options.go b/jwksauth/options.go index 13ed86a..77fe8d9 100644 --- a/jwksauth/options.go +++ b/jwksauth/options.go @@ -82,10 +82,18 @@ func WithDiscoveryTimeout(d time.Duration) Option { // yields empty fields and (when AccessRule covers those dimensions) // fails closed. // -// Surrounding whitespace is trimmed; an empty or whitespace-only string is -// treated as "use the default" (consistent with [WithVerifyTimeout]'s -// zero-input handling). Format errors are returned from [NewVerifier] / -// [NewMultiVerifier], never silently ignored. +// For [NewMultiVerifier] this prefix is shared across every configured +// issuer: the resolved server-attested key set is cached once at +// construction time. If your fleet runs multiple issuers with different +// JWT_PRIVATE_CLAIM_PREFIX values, build one [Verifier] per prefix and +// dispatch yourself rather than passing them to a single MultiVerifier. +// +// Surrounding whitespace is trimmed. Empty or whitespace-only input is a +// no-op: the option leaves the previously-configured prefix in place +// (which is the default unless an earlier WithPrivateClaimPrefix call +// set it) — it does NOT explicitly reset to the default. Format errors +// are returned from [NewVerifier] / [NewMultiVerifier], never silently +// ignored. func WithPrivateClaimPrefix(p string) Option { return optionFunc(func(c *verifierConfig) { if trimmed := strings.TrimSpace(p); trimmed != "" {