Skip to content

feat(jwksauth)!: align with upstream JWT_PRIVATE_CLAIM_PREFIX#27

Merged
appleboy merged 7 commits into
mainfrom
worktree-claim
May 3, 2026
Merged

feat(jwksauth)!: align with upstream JWT_PRIVATE_CLAIM_PREFIX#27
appleboy merged 7 commits into
mainfrom
worktree-claim

Conversation

@appleboy
Copy link
Copy Markdown
Member

@appleboy appleboy commented May 2, 2026

Summary

Aligns jwksauth with upstream AuthGate PR
#182, which moved
server-attested private claims from bare names (domain / project /
service_account) to a configurable <prefix>_<logical> scheme (default
extra_*). Adds WithPrivateClaimPrefix, replaces the previously
special-cased Tenant field/helper with a generic Claims.Extras map,
and surfaces individual values via TokenInfo.Extra(key).

This is a hard cutover; pair this SDK release with an AuthGate Server
that includes upstream PR #182.

AI Authorship

  • AI was used. Details:
    • Tool / model: Claude Opus 4.7 via Claude Code CLI (auto mode)
    • AI-authored files: every file in this PR
    • Human line-by-line reviewed: ⚠ none yet — please request a careful
      pass on jwksauth/claims.go, jwksauth/options.go,
      jwksauth/verifier.go, and jwksauth/multi_verifier.go from a
      second reviewer before merge

Change classification

  • Core code — JWT verification hot path; public API removal
    (Claims.Tenant, TokenInfo.Tenant()); on-the-wire contract change

Plan reference

See plan.md (worktree-local). Goal: align SDK consumers with the
upstream JWT_PRIVATE_CLAIM_PREFIX rollout so default-prefix tokens
continue to decode correctly, custom-prefix deployments have a knob, and
tenant is treated as a caller-supplied dimension (not a server-attested
field).

Verification

  • Unit tests — validatePrivateClaimPrefix table-driven positives & negatives
  • Integration tests — claims_prefix_test.go exercises real go-oidc
    verification against an in-process JWKS issuer
  • At least 3 e2e tests:
    - Happy path (default prefix, all three server-attested claims hit)
    - Custom prefix (acme_* hits; extra_* against an acme-configured
    verifier yields empty Domain → fail-closed)
    - Bare claim ignored (token with bare domain rejected by AccessRule;
    bare key surfaces via Extras)
    - Caller-supplied keys (extra_foo & foo both land in Extras, neither
    promotes into typed Claims)
    - Invalid prefix rejected at construction (NewVerifier and
    NewMultiVerifier)
  • Stress / soak test: N/A — no new long-running paths
  • Manual verification: stand up an AuthGate Server with PR #182 applied, run
    client_credentials, feed the issued token through the README snippet,
    confirm info.Claims.Domain / Project / ServiceAccount are populated and
    info.Extra("tenant") returns the caller-supplied value (if any).

Verifiability check

  • Inputs and outputs documented in Claims / TokenInfo /
    WithPrivateClaimPrefix godoc and jwksauth/README.md
  • Reviewer can judge correctness via signatures + tests; the decode
    path is one function (newTokenInfo) and easy to read
  • Failures surface server-side via the existing slog-based policy
    log (jwksauth: policy reject); AccessRule fails closed on missing
    values

Security check

  • No secrets in code
  • All external inputs validated (prefix format mirrors upstream
    validateJWTPrivateClaimPrefix)
  • Permission checks tested (AccessRule fail-closed reaffirmed for
    bare-key and wrong-prefix cases)
  • Rate limits applied — N/A; no new endpoints
  • Errors don't leak internals — kept the existing low-detail
    MultiVerifier.Verify error shape unchanged

Risk & rollback

  • Risk 1: SDK upgraded but server still pre-#182 → bare claims, new
    SDK reads them as empty, AccessRule blocks legitimate tokens.
    Mitigation: synchronize releases; the BREAKING flag in the commit
    signals this.
  • Risk 2: Custom-prefix deployment forgets WithPrivateClaimPrefix
    → same fail-closed symptom. Mitigation: README and godoc emphasize
    byte-for-byte agreement; test 2 pins the expected behavior.
  • Risk 3: Consumers depending on Claims.Tenant / TokenInfo.Tenant()
    break at compile time. Mitigation: 1:1 replacement via
    v, _ := info.Extra("tenant"); s, _ := v.(string); commit message and
    godoc surface this.
  • Rollback: revert the single commit on this branch — only jwksauth/
    • a one-line note in the repo CLAUDE.md are touched. Consumers who
      shipped against this SDK version with PR #182 servers will need to
      revert both ends.

Reviewer guide

  • Read carefully (line-by-line):
    • jwksauth/claims.go — the new newTokenInfo decode path: confirm the
      reserved-keys list matches upstream's staticReservedClaimKeys,
      confirm Extras population by reference is intentional, confirm
      type-assert-fail-as-empty is acceptable for non-string values
    • jwksauth/options.goWithPrivateClaimPrefix empty-string-as-default
      semantics and validatePrivateClaimPrefix rules vs. upstream
    • jwksauth/verifier.go & multi_verifier.go — prefix validation
      placement and the new claimKeys precomputation
  • Spot-check OK: the test files (mechanical bare→prefixed migration
    • new claims_prefix_test.go is well-named), README/godoc updates,
      access_rule.go (godoc-only), doc.go (godoc-only).

🤖 Generated with Claude Code

- 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 "<prefix>_domain",
"<prefix>_project", "<prefix>_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) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 2, 2026 05:40
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns jwksauth JWT-claim decoding with AuthGate’s upstream JWT_PRIVATE_CLAIM_PREFIX rollout by moving server-attested private claims to a configurable <prefix>_<logical> naming scheme (default extra_*), and replacing the prior special-cased tenant handling with a generic Claims.Extras surface.

Changes:

  • Add WithPrivateClaimPrefix + prefix validation at verifier construction; precompute resolved claim keys for the verify hot path.
  • Replace Claims.Tenant / TokenInfo.Tenant() with Claims.Extras + TokenInfo.Extra(key) and update middleware/access-rule behavior accordingly.
  • Add/update unit + integration-style tests and refresh package docs/README for the new claim/prefix contract.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
jwksauth/options.go Introduces default prefix (extra), validation rules, and WithPrivateClaimPrefix.
jwksauth/verifier.go Validates prefix at construction and passes precomputed claim keys into decoding.
jwksauth/multi_verifier.go Same prefix validation/key precompute for multi-issuer verification; updates cross-domain doc text.
jwksauth/claims.go Reworks claim decoding to read prefixed server-attested keys and collect remaining non-standard keys into Claims.Extras; adds TokenInfo.Extra.
jwksauth/access_rule.go Updates rule docs to clarify only server-attested claims participate; Extras are app-enforced.
jwksauth/access_rule_test.go Updates tests to reflect Extras behavior (no tenant field filtering).
jwksauth/middleware_test.go Migrates tests to extra_* claims and replaces tenant assertions with Extra("tenant").
jwksauth/claims_prefix_test.go Adds new tests covering default/custom prefixes, bare-claim behavior, and prefix validation.
jwksauth/doc.go Updates package-level docs to explain the private-claim prefix and Extras.
jwksauth/README.md Updates user-facing docs/examples for prefixed claims and Extras usage.
CLAUDE.md Notes the default private-claim prefix and the WithPrivateClaimPrefix override.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/middleware_test.go Outdated
Comment thread jwksauth/middleware_test.go Outdated
Comment thread jwksauth/claims_prefix_test.go Outdated
Comment thread jwksauth/README.md Outdated
Comment thread jwksauth/claims.go
- 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) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/claims.go Outdated
Comment thread jwksauth/claims.go
Comment thread jwksauth/README.md Outdated
Comment thread jwksauth/claims.go Outdated
- 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) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/options.go
- 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) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/claims.go Outdated
Comment thread jwksauth/claims_prefix_test.go Outdated
- 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) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/claims.go
Comment thread jwksauth/middleware_test.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/middleware_test.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/claims.go Outdated
Comment thread jwksauth/doc.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/options.go Outdated
Comment thread jwksauth/middleware_test.go Outdated
@appleboy appleboy requested a review from Copilot May 2, 2026 10:16
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/claims.go
Comment thread jwksauth/claims.go
Comment thread jwksauth/middleware_test.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/middleware_test.go Outdated
Comment thread jwksauth/doc.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/claims.go
Comment thread jwksauth/doc.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/middleware_test.go Outdated
Comment thread jwksauth/claims.go Outdated
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) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/claims.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread jwksauth/options.go
Comment thread jwksauth/options.go
…y-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) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 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.

@appleboy appleboy merged commit 5b43693 into main May 3, 2026
20 checks passed
yamz8 pushed a commit to kerneliushq/kernelius-forge-cli that referenced this pull request May 11, 2026
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [github.com/go-authgate/sdk-go](https://github.com/go-authgate/sdk-go) | `v0.9.0` → `v0.10.0` | ![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgo-authgate%2fsdk-go/v0.10.0?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgo-authgate%2fsdk-go/v0.9.0/v0.10.0?slim=true) |

---

### Release Notes

<details>
<summary>go-authgate/sdk-go (github.com/go-authgate/sdk-go)</summary>

### [`v0.10.0`](https://github.com/go-authgate/sdk-go/releases/tag/v0.10.0)

[Compare Source](go-authgate/sdk-go@v0.9.0...v0.10.0)

#### Changelog

##### Others

- [`5b43693`](go-authgate/sdk-go@5b43693): feat(jwksauth)!: align with upstream JWT\_PRIVATE\_CLAIM\_PREFIX ([#&#8203;27](go-authgate/sdk-go#27)) ([@&#8203;appleboy](https://github.com/appleboy))

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xNjAuNiIsInVwZGF0ZWRJblZlciI6IjQzLjE2MC42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Reviewed-on: https://gitea.com/gitea/tea/pulls/976
Co-authored-by: Renovate Bot <renovate-bot@gitea.com>
Co-committed-by: Renovate Bot <renovate-bot@gitea.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants