feat(token)!: namespace server-attested private claims under JWT_PRIVATE_CLAIM_PREFIX#182
feat(token)!: namespace server-attested private claims under JWT_PRIVATE_CLAIM_PREFIX#182
Conversation
…ATE_CLAIM_PREFIX - Add JWT_PRIVATE_CLAIM_PREFIX (default "extra") with startup validation - Replace bare claim constants with PrivateClaim registry and EmittedName helper - Compute reserved-key set at parser construction from prefix and registry - Emit prefixed keys (extra_domain, extra_project, extra_service_account) on every grant - Update discovery test to keep prefixed private claims out of claims_supported - Document the prefix and the bare-name → prefixed-name migration BREAKING CHANGE: server-attested private claims are no longer emitted under their bare names. Tokens now carry "extra_domain", "extra_project", and "extra_service_account" by default; deployments using JWT_PRIVATE_CLAIM_PREFIX=mtk emit "mtk_*". Downstream services that read claims["domain"] / claims["project"] / claims["service_account"] directly must update to the prefixed key (or the operator-chosen prefix) at the same time as the AuthGate upgrade. Token caches must be flushed on upgrade so pre-upgrade tokens with bare-name claims are not served from cache. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR introduces a deployment-configurable namespace prefix (JWT_PRIVATE_CLAIM_PREFIX, default "extra") for AuthGate server-attested private JWT claims, moving them from bare keys (domain, project, service_account) to prefixed keys (<prefix>_domain, <prefix>_project, <prefix>_service_account). It also centralizes the private-claim registry to reduce scattered wiring across emission, reserved-key checks, and tests.
Changes:
- Add
JWT_PRIVATE_CLAIM_PREFIXconfig support, defaults, and startup validation (shape/length/trailing_/collision guard). - Introduce
token.PrivateClaimsregistry andtoken.EmittedName(prefix, logical)helper; update issuance paths to emit prefixed keys. - Update extra-claims reserved-key derivation and parser logic to be prefix-aware; refresh/flow tests and docs updated accordingly.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/config/config.go | Adds default/validation for JWT_PRIVATE_CLAIM_PREFIX, including regex/length and collision checks. |
| internal/config/config_test.go | Ensures base test config includes the new prefix default. |
| internal/token/types.go | Adds PrivateClaims registry and EmittedName composition helper. |
| internal/token/reserved.go | Reworks reserved-claim handling to be prefix-derived via BuildReservedClaimKeys and updates ValidateExtraClaims signature. |
| internal/token/reserved_test.go | Updates reserved-key and ValidateExtraClaims unit tests for prefix behavior. |
| internal/services/token.go | Emits client/server private claims using the configured prefix during issuance composition. |
| internal/services/token_test.go | Sets default prefix for tests that construct Config{} ad-hoc. |
| internal/services/token_extra_claims_parser.go | Precomputes reserved-key set once per parser instance and validates using it. |
| internal/services/token_extra_claims_parser_test.go | Ensures parser tests use default prefix config. |
| internal/services/token_extra_claims_test.go | Updates claim-building and server-override tests to expect prefixed keys. |
| internal/services/token_domain_test.go | Updates domain-claim tests to validate prefixed domain emission/omission. |
| internal/services/token_private_claim_prefix_test.go | Adds an end-to-end-ish test matrix for default/custom prefix behavior and validation. |
| internal/token/local_test.go | Updates provider injection tests to use prefixed keys. |
| internal/handlers/oidc_test.go | Ensures discovery claims_supported doesn’t list private claims (bare or prefixed default). |
| docs/JWT_VERIFICATION.md | Updates verification/migration guidance to prefixed private-claim keys. |
| docs/CONFIGURATION.md | Documents new env var, validation rules, and migration notes. |
| .env.example | Documents new env var, breaking change, and cache-flush guidance. |
| CLAUDE.md | Updates configuration reference to include the new prefix behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replaces vendor-specific MTK references with the industry-standard fictional placeholder "acme" across docs, comments, and test fixtures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add bare logical names (domain/project/service_account) to BuildReservedClaimKeys so callers cannot smuggle legacy claim names past the parser - Strip bare logical names in generateJWT as defense-in-depth so the strip survives any future parser bypass - Default empty cfg.JWTPrivateClaimPrefix to DefaultJWTPrivateClaimPrefix in NewExtraClaimsParser to match production semantics for ad-hoc test configs - Use DefaultJWTPrivateClaimPrefix constant in Load() instead of the literal "extra" - Document PrivateClaims as read-only at runtime - Rename TestJWTPrivateClaimPrefix_CollisionRejected to reflect what it asserts and add a real synthetic-injection collision test in the config package - Add regression test locking in that bare legacy names cannot land in the signed JWT Addresses Copilot review feedback on PR #182. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… list - Unexport the private-claim registry as privateClaims and expose a defensive copy via PrivateClaimRegistry() so external packages cannot mutate the slice at runtime - Precompute generateJWTStripList at package init so generateJWT no longer rebuilds the bare-name denylist on every issued token - Iterate privateClaims in BuildReservedClaimKeys's bare-name test instead of hardcoding the legacy names, so adding a future PrivateClaim entry automatically keeps coverage in lockstep - Sync docs/CONFIGURATION.md and CLAUDE.md with the actual reserved set, which now also includes the bare logical names plus the strip-at-sign defense Addresses Copilot review feedback on PR #182. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…loyments
- Always reserve the default-prefixed forms (extra_domain / extra_project / extra_service_account) in BuildReservedClaimKeys, even when the configured prefix is custom, so callers cannot smuggle them past the parser into the signed JWT
- Move the generateJWT strip list onto LocalTokenProvider (computed once at NewLocalTokenProvider) so it can be config-aware: when the configured prefix differs from the default, also strip the default-prefixed forms as defense-in-depth
- Default an empty JWTPrivateClaimPrefix to DefaultJWTPrivateClaimPrefix in NewLocalTokenProvider to match production semantics for ad-hoc Config{} test fixtures
- Eliminate the parallel staticReservedClaimKeys list in internal/token by exporting StaticReservedClaimKeys from internal/config (the leaf-most package, since token already imports config) so the runtime reserved-key derivation and the startup collision check share a single source of truth
- Add regression test locking in that callers cannot inject default-prefixed claims on a custom-prefix deployment
Addresses Copilot review feedback on PR #182.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ording - Unexport StaticReservedClaimKeys as staticReservedClaimKeys + add a StaticReservedClaimKeys() accessor returning a defensive copy, so other packages cannot mutate the canonical reserved-claim list at runtime - Rephrase the AuthGate-emitted private-claim registry: only <prefix>_domain is server-attested; <prefix>_project and <prefix>_service_account are owner-set per-client metadata (untrusted assertions). Updated PrivateClaim and privateClaims doc comments, .env.example, CLAUDE.md, docs/CONFIGURATION.md, and docs/JWT_VERIFICATION.md to reserve "server-attested" specifically for domain and use "AuthGate-emitted" / "AuthGate-private" for the broader category Addresses Copilot review feedback on PR #182. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Convert TestDiscovery_OmitsDomainEvenWhenSet to a table-driven test that also exercises an explicit default prefix and a custom-prefix deployment (acme), so the "AuthGate-private claim must not appear in claims_supported" invariant is checked under every prefix shape. Also tighten the comment in TestDiscovery_ReturnsCorrectMetadata to reflect what the assertion list actually covers, pointing the custom-prefix coverage at the dedicated test. Addresses Copilot review feedback on PR #182. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…obal mutation Extract the prefix-vs-reserved-key collision check from validateJWTPrivateClaimPrefix into a pure detectPrefixCollision helper that takes the logical-name and reserved-key lists as arguments. The synthetic collision test now calls the helper directly with a one-off list, so it no longer mutates the package-level jwtPrivateClaimLogicalNames slice and is safe under t.Parallel() and concurrent Validate() calls. Also stop mutating the caller-provided *config.Config in createTestTokenService — apply the default prefix on a shallow copy so fixtures shared across subtests don't observe a side-effected prefix. Addresses Copilot review feedback on PR #182. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ateExtraClaims
- Make Config.Validate()'s prefix check treat an empty JWTPrivateClaimPrefix as "use the default" instead of erroring, matching the behavior already applied by Load() and the runtime layers (NewExtraClaimsParser, NewLocalTokenProvider, NewTokenService). Removes the inconsistency where ad-hoc Config{} fixtures would surface an empty-prefix error from Validate while the rest of the codebase silently substituted the default.
- ValidateExtraClaims now rejects a nil reserved map up front. A nil reserved map would silently disable reserved-key enforcement (nil-map lookups return ok=false), creating a footgun for future internal callers — explicit rejection makes the contract impossible to misuse.
Addresses Copilot review feedback on PR #182.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…x reservation Address Copilot review comments: 1. Add TestPrivateClaimRegistryDrift (drift_test.go) that fails the build if config.jwtPrivateClaimLogicalNames diverges from token.privateClaims. 2. Document in CONFIGURATION.md that the default-prefixed forms (extra_domain, extra_project, extra_service_account) are always reserved even when JWT_PRIVATE_CLAIM_PREFIX is set to a custom value. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…pKeys Address Copilot review comments: 1. Reword local.go comment to accurately describe why prefixed private claims survive: computeStripList excludes them, not "written after". 2. Rename oidcStripKeys → jwtStripKeys since it contains both RFC 7519 registered claims (nbf) and OIDC ID-token claims. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… slice capacity Address Copilot review comments: 1. Replace hardcoded "extra" with config.DefaultJWTPrivateClaimPrefix in TestBuildClientClaims to avoid test drift. 2. Size computeStripList capacity to 3*len(privateClaims) to account for the non-default-prefix branch without reallocation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eld doc
Document that empty JWTPrivateClaimPrefix is normalized to the default
("extra") at validation time, so operators and test authors are not
confused by the "1–15 chars" constraint.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- JWTDomain: clarify emitted key is "<prefix>_domain" not bare "domain" - JWTPrivateClaimPrefix: clarify empty normalization is caller-side (Validate checks but does not mutate the field) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Introduces
JWT_PRIVATE_CLAIM_PREFIX(default"extra") — a configurable namespace AuthGate prepends (with an_separator AuthGate adds itself) to every server-attested private JWT claim. With the default, JWTs now carryextra_domain/extra_project/extra_service_accountinstead of baredomain/project/service_account. A deployment usingJWT_PRIVATE_CLAIM_PREFIX=acmewould emitacme_*. The bare logical names are gone — this is a hard cutover, called out below as a breaking change.The new claim list lives in an unexported registry inside
internal/tokenexposed viatoken.PrivateClaimRegistry()(returns a defensive copy). Future additions are a one-line append rather than scattered edits across emission, reserved-keys, and tests.Server-attested private claims are no longer emitted under their bare names. Downstream services that read
claims["domain"]/claims["project"]/claims["service_account"]directly must update at the same time as the AuthGate upgrade:Token cache flush required. Tokens minted before the upgrade may sit in the cache with bare-name claims. Flush the token cache (Redis FLUSH or restart with empty memory cache) on upgrade, or wait for tokens to expire naturally. There is no automatic cache-key namespace bump.
AI Authorship
plan.mdinternal/config/config.go(validation),internal/token/reserved.go(reserved-set logic),internal/services/token.go(buildClientClaims/buildServerClaims/composeIssuanceClaims), andinternal/services/token_extra_claims_parser.go.Change classification
This change alters the wire format of every issued JWT in every grant flow (
authorization_code,device_code,client_credentials,refresh_token). Failure or regression spreads to every downstream consumer of AuthGate tokens. Reviewers should treat the issuance and reserved-key paths as security-critical.Plan reference
Implementation followed
plan.mdon the branch. Goal: introduce a configurable prefix so downstream services can syntactically distinguish AuthGate-private claims from OIDC public claims, removing the per-deployment lookup table. Hard cutover by design.Verification
extra_domain/extra_project/extra_service_account(when configured) and bare names are absent.JWT_PRIVATE_CLAIM_PREFIX=acme, restart → issue token → confirmacme_*keys,extra_*and bare names absent./oauth/tokenwithextra_claims={"extra_domain":"evil"}→ expect 400invalid_request.curl /.well-known/openid-configuration→ confirmclaims_supporteddoes not listextra_domain(or any<prefix>_*private claim).extra_,1bad, 16-char) → expect startup failure with a clear message.Test coverage added in
internal/services/token_private_claim_prefix_test.go:TestPrivateClaimPrefix_DefaultPrefix_HappyPathclient_credentialswith default prefix; bare names absentTestPrivateClaimPrefix_CustomPrefixacmeprefix;extra_*and bare names absentTestPrivateClaimPrefix_CallerCannotImpersonatePrefixedClaimTestPrivateClaimPrefix_CallerCannotInjectBareLegacyNamedomain/project/service_accountblocked at parser AND stripped at sign timeTestPrivateClaimPrefix_CallerCannotInjectDefaultPrefixOnCustomDeploymentprefix=acme, smuggledextra_*blocked at parser AND stripped at sign timeTestPrivateClaimPrefix_RefreshContinuityTestPrivateClaimPrefix_RegistryTableDrivenEmittedNamecomposes consistently for every entry returned byPrivateClaimRegistry()TestJWTPrivateClaimPrefix_StartupValidation_/ oversized prefixTestDetectPrefixCollision_Synthetic(config package)Existing tests (
token_domain_test.go,token_extra_claims_test.go,local_test.go,reserved_test.go,oidc_test.go) were updated to use the prefixed names from config rather than the deleted constants.make generate && make fmt && make lint && make testare all green.Verifiability check
.env.example,docs/CONFIGURATION.md,docs/JWT_VERIFICATION.md,CLAUDE.mdEmittedName,BuildReservedClaimKeys,composeIssuanceClaims)Config.Validate()) with a clear message naming the bad prefix and the conflicting keySecurity check
<prefix>_<logical>for every registry entry, (3) the bare logical names (domain/project/service_account) so callers cannot smuggle the legacy pre-prefix keys past it, and (4) the default-prefixed forms (extra_*) — reserved even on custom-prefix deployments so an un-migrated downstream consumer hardcoded to the default cannot be tricked.generateJWTstrips the OIDC ID-token-only keys (nbf/azp/amr/acr/auth_time/nonce/at_hash), the bare logical names, and (on custom-prefix deployments) the default-prefixed forms — so a future regression that bypasses the parser still cannot land any of these in the signed JWT.<prefix>_domainstill wins on collision:applyServerClaimswrites it last, overriding caller- or client-supplied values.claims_supportedexcludes bare AND prefixed AuthGate-private claim names under default and custom prefixes (oidc_test.goexercises all three configurations).JWT_DOMAINsemantics unchanged (server-attested, validated at startup, re-resolved on refresh).Defense-in-depth coverage:
TestPrivateClaimPrefix_CallerCannotImpersonatePrefixedClaim— parser rejection + service-layer override for the configured prefix's composed key.TestPrivateClaimPrefix_CallerCannotInjectBareLegacyName— bare names blocked at both layers.TestPrivateClaimPrefix_CallerCannotInjectDefaultPrefixOnCustomDeployment—extra_*blocked at both layers when configured prefix is non-default.Risk & rollback
Risk
claims["domain"]. Mitigation: prominent migration note in.env.example/docs/CONFIGURATION.md/docs/JWT_VERIFICATION.md/CLAUDE.mdand this PR description; downstream integration smoke tests should run before merge.Config.Validate()fails fast with a clear message naming the bad value or the colliding composed key.Rollback
Revert the PR. Tokens minted under the new code carry only prefixed claims; downstream services that already migrated will need a coordinated revert. Treat this as a one-way door once consumers adopt — the practical rollback window is short.
Reviewer guide
Read carefully (line-by-line):
internal/config/config.go—validateJWTPrivateClaimPrefix(regex / length / trailing-_/ collision check) and the static reserved-keys list. Note the documented sync requirement withinternal/token/reserved.go(the import cycle prevents sharing).internal/token/reserved.go—BuildReservedClaimKeyssemantics;ValidateExtraClaimsnow takes a precomputed reserved set rather than reading a package-level map.internal/services/token.go—buildClientClaims(client, prefix),buildServerClaims(cfg),composeIssuanceClaims(precedence caller → client → server unchanged).internal/services/token_extra_claims_parser.go—NewExtraClaimsParserprecomputes the reserved set at construction time using the configured prefix.internal/token/types.go— unexportedprivateClaimsregistry +PrivateClaimRegistry()accessor (returns a defensive copy) +EmittedName(prefix, logical)helper. Single source of truth for key composition; mutation from other packages is impossible by construction.Spot-check OK:
token_private_claim_prefix_test.gomatrix tell the story)..env.example,docs/CONFIGURATION.md,docs/JWT_VERIFICATION.md,CLAUDE.md.Suggested reviewer count: 2+ (core code) — including the token / OAuth module owner.
🤖 Generated with Claude Code