From 9e950cf97195db3e109fbfffa44eca6f49b9cfbb Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 17:55:46 -0700 Subject: [PATCH 1/4] refactor(proof/oidc): godocs on private types and helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add concise godocs to every private type, struct, and helper the package exposes internally: options/defaultOptions, cachedKeySet, keySetRequest, the source.go clone family, claimPathKey, cloneClaimValue, validateHTTPSURL, signingAlgorithms, unverifiedIssuer, audienceAllowed, forwardClaims, mergedForwardedClaims, tokenClaim, setClaim, keySet, keySetCacheKey, fetchKeySet, constrainKeySet, verificationKey, keyAllowsVerification, and unauthenticated. Several godocs name the security invariant they preserve — HTTPS-only issuer/JWKS URLs, symmetric-algorithm refusal, maxJWKSBytes cap, forwarded-claim allow-list, fail-closed empty-key-set check, JWK use/key_ops gating — to match the inline-comment style about to land on Verifier.VerifyToken. Co-Authored-By: Claude Opus 4.7 (1M context) --- proof/oidc/options.go | 2 ++ proof/oidc/source.go | 9 +++++++++ proof/oidc/types.go | 7 +++++++ proof/oidc/verifier.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+) diff --git a/proof/oidc/options.go b/proof/oidc/options.go index dcd0b9e..c39191e 100644 --- a/proof/oidc/options.go +++ b/proof/oidc/options.go @@ -12,6 +12,7 @@ const ( defaultKeySetCacheTTL = 5 * time.Minute ) +// options is the resolved configuration consumed by a Verifier. type options struct { httpClient *http.Client clock func() time.Time @@ -23,6 +24,7 @@ type options struct { // Option configures a Verifier. type Option func(*options) +// defaultOptions returns the baseline options applied before any caller-supplied Option. func defaultOptions() options { return options{ httpClient: &http.Client{Timeout: defaultHTTPTimeout}, diff --git a/proof/oidc/source.go b/proof/oidc/source.go index 0378294..ca764e6 100644 --- a/proof/oidc/source.go +++ b/proof/oidc/source.go @@ -64,6 +64,8 @@ func (s *StaticProviderSource) FindProvider(ctx context.Context, issuer string) return cloneProvider(provider), nil } +// cloneProvider returns a deep copy of provider so callers cannot mutate the +// audience, signing-algorithm, or forwarded-claim slices the source retains. func cloneProvider(provider Provider) Provider { provider.Audiences = cloneStrings(provider.Audiences) provider.SupportedSigningAlgorithms = cloneStrings(provider.SupportedSigningAlgorithms) @@ -72,6 +74,7 @@ func cloneProvider(provider Provider) Provider { return provider } +// cloneStrings returns a defensive copy of values, returning nil for empty input. func cloneStrings(values []string) []string { if len(values) == 0 { return nil @@ -83,6 +86,7 @@ func cloneStrings(values []string) []string { return cloned } +// cloneClaimPaths returns a deep copy of paths, returning nil for empty input. func cloneClaimPaths(paths []authkit.ClaimPath) []authkit.ClaimPath { if len(paths) == 0 { return nil @@ -96,6 +100,7 @@ func cloneClaimPaths(paths []authkit.ClaimPath) []authkit.ClaimPath { return cloned } +// cloneClaimPath returns a defensive copy of path, returning nil for empty input. func cloneClaimPath(path authkit.ClaimPath) authkit.ClaimPath { if len(path) == 0 { return nil @@ -107,6 +112,8 @@ func cloneClaimPath(path authkit.ClaimPath) authkit.ClaimPath { return cloned } +// claimPathKey serializes path into a deterministic dedup key. Returns the empty +// string when path is invalid so the caller can skip it. func claimPathKey(path authkit.ClaimPath) string { if !path.Valid() { return "" @@ -115,6 +122,8 @@ func claimPathKey(path authkit.ClaimPath) string { return strings.Join(path, "\x00") } +// cloneClaimValue returns a deep copy of value covering the JSON-decoded shapes +// that token claims can carry. Other scalars are returned as-is. func cloneClaimValue(value any) any { switch typed := value.(type) { case map[string]any: diff --git a/proof/oidc/types.go b/proof/oidc/types.go index f4d7f43..fb230c5 100644 --- a/proof/oidc/types.go +++ b/proof/oidc/types.go @@ -64,6 +64,9 @@ func (p Provider) Validate() error { return nil } +// validateHTTPSURL rejects raw values that are empty, contain surrounding whitespace, +// or are not absolute https:// URLs with a host. The HTTPS requirement defends +// against issuer or JWKS configuration that would otherwise allow on-path tampering. func validateHTTPSURL(name string, raw string) error { if strings.TrimSpace(raw) == "" { return fmt.Errorf("oidc: provider %s is required", name) @@ -80,6 +83,10 @@ func validateHTTPSURL(name string, raw string) error { return nil } +// signingAlgorithms resolves the provider's SupportedSigningAlgorithms (or the +// RS256 default) to typed [jwa.SignatureAlgorithm] values, rejecting unknown, +// empty, or symmetric algorithms. Symmetric algorithms are refused because they +// would let an attacker who obtains the JWKS verifier key impersonate the issuer. func (p Provider) signingAlgorithms() ([]jwa.SignatureAlgorithm, error) { names := p.SupportedSigningAlgorithms if len(names) == 0 { diff --git a/proof/oidc/verifier.go b/proof/oidc/verifier.go index b4f4f5e..f8e692c 100644 --- a/proof/oidc/verifier.go +++ b/proof/oidc/verifier.go @@ -37,11 +37,14 @@ type Verifier struct { requests map[string]*keySetRequest } +// cachedKeySet pairs a fetched JWKS with the time it ceases to be reusable. type cachedKeySet struct { set jwk.Set expiresAt time.Time } +// keySetRequest carries the result of an in-flight JWKS fetch so concurrent +// callers can wait for the same provider's keys without duplicating the request. type keySetRequest struct { done chan struct{} set jwk.Set @@ -138,6 +141,10 @@ func (v *Verifier) VerifyToken(ctx context.Context, rawToken string) (authkit.Id return identity, nil } +// unverifiedIssuer extracts the iss claim from rawToken without verifying its +// signature. Used to look up the trusted provider before the verifying parse +// runs; the same iss value is rechecked against the resolved provider before +// the token is honored. func unverifiedIssuer(rawToken string) (string, error) { token, err := jwt.ParseInsecure([]byte(rawToken)) if err != nil { @@ -152,6 +159,9 @@ func unverifiedIssuer(rawToken string) (string, error) { return issuer, nil } +// audienceAllowed reports whether any value in token's aud claim is in the +// resource server's accepted audiences. Returns false for tokens with no +// audience claim so an absent aud cannot accidentally pass the gate. func audienceAllowed(token jwt.Token, audiences []string) bool { tokenAudiences, ok := token.Audience() if !ok || len(tokenAudiences) == 0 { @@ -171,6 +181,9 @@ func audienceAllowed(token jwt.Token, audiences []string) bool { return false } +// forwardClaims projects the verified token's claims into the subset selected +// by the verifier and provider configuration. Only paths in the merged +// allow-list reach authkit.Identity.Claims so untrusted claims never leak. func (v *Verifier) forwardClaims(token jwt.Token, provider Provider) map[string]any { paths := mergedForwardedClaims(provider.ForwardedClaims, v.forwardedClaims) if len(paths) == 0 { @@ -195,6 +208,9 @@ func (v *Verifier) forwardClaims(token jwt.Token, provider Provider) map[string] return claims } +// mergedForwardedClaims returns the deduplicated union of the provider's +// forwarded-claim allow-list and the verifier's static allow-list, preserving +// provider order then static order. Invalid paths are dropped. func mergedForwardedClaims( providerClaims []authkit.ClaimPath, staticClaims []authkit.ClaimPath, @@ -221,6 +237,8 @@ func mergedForwardedClaims( return claims } +// tokenClaim returns the value at path inside token's verified claims, deep-copying +// the result so subsequent mutations cannot affect the underlying token. func tokenClaim(token jwt.Token, path authkit.ClaimPath) (any, bool) { var value any if err := token.Get(path[0], &value); err != nil { @@ -241,6 +259,8 @@ func tokenClaim(token jwt.Token, path authkit.ClaimPath) (any, bool) { return cloneClaimValue(resolved), true } +// setClaim writes value into claims under the nested path, creating intermediate +// maps as needed. Non-map intermediate values are overwritten with a fresh map. func setClaim(claims map[string]any, path authkit.ClaimPath, value any) { current := claims for _, segment := range path[:len(path)-1] { @@ -255,6 +275,9 @@ func setClaim(claims map[string]any, path authkit.ClaimPath, value any) { current[path[len(path)-1]] = value } +// keySet returns the cached JWKS for provider when fresh, coalesces concurrent +// fetches so multiple verifications never hammer the JWKS endpoint at once, and +// caches successful results until keySetCacheTTL elapses. func (v *Verifier) keySet(ctx context.Context, provider Provider) (jwk.Set, error) { now := v.clock() cacheKey, err := keySetCacheKey(provider) @@ -299,6 +322,9 @@ func (v *Verifier) keySet(ctx context.Context, provider Provider) (jwk.Set, erro return req.set, req.err } +// keySetCacheKey derives the cache key for a provider's JWKS by combining the +// JWKS URL with the resolved signing-algorithm allow-list, so an algorithm +// reconfiguration invalidates any previously-cached keys. func keySetCacheKey(provider Provider) (string, error) { algorithms, err := provider.signingAlgorithms() if err != nil { @@ -313,6 +339,9 @@ func keySetCacheKey(provider Provider) (string, error) { return provider.JWKSURL + "\x00" + strings.Join(names, ","), nil } +// fetchKeySet retrieves and constrains the JWKS at provider.JWKSURL. The body +// is capped at maxJWKSBytes so a hostile or oversized response cannot exhaust +// memory, and only keys allowed by the provider's signing-algorithm list survive. func (v *Verifier) fetchKeySet(ctx context.Context, provider Provider) (jwk.Set, error) { httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, provider.JWKSURL, nil) if err != nil { @@ -349,6 +378,9 @@ func (v *Verifier) fetchKeySet(ctx context.Context, provider Provider) (jwk.Set, return constrained, nil } +// constrainKeySet narrows set to the public verification keys allowed by the +// provider's signing-algorithm list. Returns an error when no keys survive so +// the caller fails closed rather than verifying against an empty key set. func constrainKeySet(set jwk.Set, provider Provider) (jwk.Set, error) { allowed, err := provider.signingAlgorithms() if err != nil { @@ -384,6 +416,10 @@ func constrainKeySet(set jwk.Set, provider Provider) (jwk.Set, error) { return constrained, nil } +// verificationKey returns key as a public verification key when the JWKS entry +// is allowed for token verification under the provider's signing-algorithm list. +// Keys with explicit non-matching alg, or unallowed use/key_ops, are skipped. +// The single-allowed-algorithm branch sets alg on the key so jwt.Parse can find it. func verificationKey( key jwk.Key, allowed []jwa.SignatureAlgorithm, @@ -419,6 +455,10 @@ func verificationKey( return publicKey, true, nil } +// keyAllowsVerification reports whether key's JWK use/key_ops declare it usable +// for signature verification. Keys explicitly marked for encryption or omitting +// the verify key_op are refused so a JWKS that mixes purposes cannot smuggle a +// key intended for a different operation into the verification path. func keyAllowsVerification(key jwk.Key) bool { if usage, ok := key.KeyUsage(); ok && usage != jwk.ForSignature.String() { return false @@ -432,6 +472,8 @@ func keyAllowsVerification(key jwk.Key) bool { return slices.Contains(ops, jwk.KeyOpVerify) } +// unauthenticated wraps reason as an [authkit.ErrUnauthenticated] so callers can +// detect auth failures with [errors.Is] without inspecting message strings. func unauthenticated(reason string) error { return fmt.Errorf("%w: %s", authkit.ErrUnauthenticated, reason) } From 9eac2fcdbb85e84c00f3ddac80b8d80673641232 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 17:56:45 -0700 Subject: [PATCH 2/4] refactor(proof/oidc): security inline comments on Verifier.VerifyToken Annotate each gate in the OIDC JWT trust path so the security invariant it preserves is explicit. Mirrors the access/jwt/verifier.go style from PR #61. The annotations cover: empty-token rejection (signal amplification), unverified-issuer partial parse (lookup-then-verify rationale), trusted issuer lookup (untrusted-issuer defense), provider-issuer consistency (config-swap defense), revalidation of the resolved provider (corrupt trust-store row), the jwt.Parse omnibus (forgery, expiration bypass, time confusion, key-use smuggling), explicit non-empty subject check (blank-subject confusion), and audience match (audience confusion). Co-Authored-By: Claude Opus 4.7 (1M context) --- proof/oidc/verifier.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/proof/oidc/verifier.go b/proof/oidc/verifier.go index f8e692c..18a09e3 100644 --- a/proof/oidc/verifier.go +++ b/proof/oidc/verifier.go @@ -77,28 +77,46 @@ func NewVerifier(source ProviderSource, opts ...Option) (*Verifier, error) { } // VerifyToken verifies rawToken and returns its OIDC identity. +// +// The function is the trust gate for externally-issued tokens. Each branch +// below carries a comment naming the attack class it defends against so the +// gate cannot be relaxed accidentally. func (v *Verifier) VerifyToken(ctx context.Context, rawToken string) (authkit.Identity, error) { if err := ctx.Err(); err != nil { return authkit.Identity{}, err } + // Reject empty tokens up front so the unverified-issuer parse never sees + // input that could amplify a nil-handling bug downstream. if rawToken == "" { return authkit.Identity{}, unauthenticated("token is required") } + // Extract iss without verifying the signature so we can look up the trusted + // provider config. The same iss value is rechecked against provider.Issuer + // below and is also enforced inside jwt.Parse via jwt.WithIssuer, so an + // attacker cannot use a forged iss to slip past the gate. issuer, err := unverifiedIssuer(rawToken) if err != nil { return authkit.Identity{}, err } provider, err := v.source.FindProvider(ctx, issuer) + // An unknown issuer is the untrusted-issuer defense: only tokens whose iss + // matches a configured trust record are considered. if errors.Is(err, ErrProviderNotFound) { return authkit.Identity{}, unauthenticated("issuer is not trusted") } if err != nil { return authkit.Identity{}, fmt.Errorf("oidc: find provider: %w", err) } + // Defend against a misbehaving ProviderSource that returns a record for a + // different issuer than the one requested (config swap or storage bug): + // refuse rather than verify against unrelated trust data. if provider.Issuer != issuer { return authkit.Identity{}, unauthenticated("provider issuer mismatch") } + // Re-validate the resolved provider so a corrupt trust-store row cannot + // disable HTTPS-only, audience, or algorithm checks just because it was + // written without going through the canonical Validate path. if validationErr := provider.Validate(); validationErr != nil { return authkit.Identity{}, fmt.Errorf("oidc: invalid provider: %w", validationErr) } @@ -108,6 +126,11 @@ func (v *Verifier) VerifyToken(ctx context.Context, rawToken string) (authkit.Id return authkit.Identity{}, err } + // jwt.Parse enforces signature verification (forgery), required exp claim + // plus AcceptableSkew (expiration bypass / time-confusion), required sub + // claim, and issuer match against the resolved provider. The verifying + // parse runs against the constrained key set so an attacker cannot smuggle + // in a JWKS key bound to a different algorithm or use. token, err := jwt.Parse( []byte(rawToken), jwt.WithKeySet(set), @@ -121,10 +144,15 @@ func (v *Verifier) VerifyToken(ctx context.Context, rawToken string) (authkit.Id return authkit.Identity{}, unauthenticated("JWT verification failed") } + // jwt.Parse already requires the subject claim, but an explicit non-empty + // check defends against a present-but-blank sub that would otherwise + // produce an identity addressing no external account. subject, ok := token.Subject() if !ok || subject == "" { return authkit.Identity{}, unauthenticated("subject claim is required") } + // Audience-confusion defense: refuse tokens whose aud claim does not match + // any audience this resource server has been told to accept. if !audienceAllowed(token, provider.Audiences) { return authkit.Identity{}, unauthenticated("audience is not accepted") } From ca75c5539035dbe7b3dbc9523ea7abd8297bc0ab Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 17:58:53 -0700 Subject: [PATCH 3/4] test(proof/oidc): split verifier_test.go per domain Lift the 616-LOC verifier_test.go into four files mirroring the production layout: provider_test.go (Provider.Validate), source_test.go (StaticProviderSource), verifier_test.go (Verifier.VerifyToken only), helpers_test.go (fixtures + testIssuer + tokenRequest + failingProviderSource + fixedTime). No test logic changes; each function is lifted verbatim. Co-Authored-By: Claude Opus 4.7 (1M context) --- proof/oidc/helpers_test.go | 168 ++++++++++++++++++++++ proof/oidc/provider_test.go | 88 ++++++++++++ proof/oidc/source_test.go | 53 +++++++ proof/oidc/verifier_test.go | 277 ------------------------------------ 4 files changed, 309 insertions(+), 277 deletions(-) create mode 100644 proof/oidc/helpers_test.go create mode 100644 proof/oidc/provider_test.go create mode 100644 proof/oidc/source_test.go diff --git a/proof/oidc/helpers_test.go b/proof/oidc/helpers_test.go new file mode 100644 index 0000000..6f6ba09 --- /dev/null +++ b/proof/oidc/helpers_test.go @@ -0,0 +1,168 @@ +package oidc_test + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/lestrrat-go/jwx/v3/jwa" + "github.com/lestrrat-go/jwx/v3/jwk" + "github.com/lestrrat-go/jwx/v3/jwt" + "github.com/stretchr/testify/require" + + authkitoidc "github.com/meigma/authkit/proof/oidc" +) + +const ( + testAudience = "authkit-api" + testSubject = "user-123" +) + +type failingProviderSource struct { + err error +} + +func (s failingProviderSource) FindProvider(context.Context, string) (authkitoidc.Provider, error) { + return authkitoidc.Provider{}, s.err +} + +type testIssuer struct { + server *httptest.Server + issuer string + jwksURL string + signingKey jwk.Key + publicSet jwk.Set +} + +type tokenRequest struct { + issuer string + subject string + audiences []string + expiresAt time.Time + notBefore *time.Time + jwtID string + claims map[string]any +} + +func newTestIssuer(t *testing.T) *testIssuer { + t.Helper() + + return newTestIssuerWithPublicKey(t, nil) +} + +func newTestIssuerWithPublicKey(t *testing.T, configure func(*testing.T, jwk.Key)) *testIssuer { + t.Helper() + + rawKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + signingKey, err := jwk.Import(rawKey) + require.NoError(t, err) + require.NoError(t, signingKey.Set(jwk.KeyIDKey, "test-key")) + require.NoError(t, signingKey.Set(jwk.AlgorithmKey, jwa.RS256())) + + privateSet := jwk.NewSet() + require.NoError(t, privateSet.AddKey(signingKey)) + publicSet, err := jwk.PublicSetOf(privateSet) + require.NoError(t, err) + if configure != nil { + publicKey, ok := publicSet.Key(0) + require.True(t, ok) + configure(t, publicKey) + } + + issuer := &testIssuer{ + signingKey: signingKey, + publicSet: publicSet, + } + mux := http.NewServeMux() + mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(issuer.publicSet); err != nil { + t.Errorf("encode JWKS: %v", err) + } + }) + mux.HandleFunc("/unavailable", func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + }) + issuer.server = httptest.NewTLSServer(mux) + t.Cleanup(issuer.server.Close) + issuer.issuer = issuer.server.URL + issuer.jwksURL = issuer.server.URL + "/jwks" + + return issuer +} + +func (i *testIssuer) provider() authkitoidc.Provider { + return authkitoidc.Provider{ + Issuer: i.issuer, + Audiences: []string{testAudience}, + JWKSURL: i.jwksURL, + } +} + +func (i *testIssuer) verifier(t *testing.T, opts ...authkitoidc.Option) *authkitoidc.Verifier { + t.Helper() + + opts = append([]authkitoidc.Option{authkitoidc.WithHTTPClient(i.server.Client())}, opts...) + + return newVerifier(t, i.provider(), opts...) +} + +func (i *testIssuer) sign(t *testing.T, req tokenRequest) string { + t.Helper() + + issuer := req.issuer + if issuer == "" { + issuer = i.issuer + } + builder := jwt.NewBuilder(). + Issuer(issuer). + Audience(req.audiences). + IssuedAt(fixedTime().Add(-time.Minute)) + if req.subject != "" { + builder.Subject(req.subject) + } + if !req.expiresAt.IsZero() { + builder.Expiration(req.expiresAt) + } + if req.notBefore != nil { + builder.NotBefore(*req.notBefore) + } + if req.jwtID != "" { + builder.JwtID(req.jwtID) + } + for name, value := range req.claims { + builder.Claim(name, value) + } + + token, err := builder.Build() + require.NoError(t, err) + signed, err := jwt.Sign(token, jwt.WithKey(jwa.RS256(), i.signingKey)) + require.NoError(t, err) + + return string(signed) +} + +func newVerifier( + t *testing.T, + provider authkitoidc.Provider, + opts ...authkitoidc.Option, +) *authkitoidc.Verifier { + t.Helper() + + source, err := authkitoidc.NewStaticProviderSource(provider) + require.NoError(t, err) + verifier, err := authkitoidc.NewVerifier(source, opts...) + require.NoError(t, err) + + return verifier +} + +func fixedTime() time.Time { + return time.Date(2026, 5, 8, 12, 0, 0, 0, time.UTC) +} diff --git a/proof/oidc/provider_test.go b/proof/oidc/provider_test.go new file mode 100644 index 0000000..9925c60 --- /dev/null +++ b/proof/oidc/provider_test.go @@ -0,0 +1,88 @@ +package oidc_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + authkitoidc "github.com/meigma/authkit/proof/oidc" +) + +func TestProviderValidation(t *testing.T) { + tests := []struct { + name string + provider authkitoidc.Provider + }{ + { + name: "missing issuer", + provider: authkitoidc.Provider{ + Audiences: []string{testAudience}, + JWKSURL: "https://issuer.example/jwks", + }, + }, + { + name: "missing audience", + provider: authkitoidc.Provider{ + Issuer: "https://issuer.example", + JWKSURL: "https://issuer.example/jwks", + }, + }, + { + name: "missing JWKS URL", + provider: authkitoidc.Provider{ + Issuer: "https://issuer.example", + Audiences: []string{testAudience}, + }, + }, + { + name: "insecure issuer", + provider: authkitoidc.Provider{ + Issuer: "http://issuer.example", + Audiences: []string{testAudience}, + JWKSURL: "https://issuer.example/jwks", + }, + }, + { + name: "relative issuer", + provider: authkitoidc.Provider{ + Issuer: "/issuer", + Audiences: []string{testAudience}, + JWKSURL: "https://issuer.example/jwks", + }, + }, + { + name: "insecure JWKS URL", + provider: authkitoidc.Provider{ + Issuer: "https://issuer.example", + Audiences: []string{testAudience}, + JWKSURL: "http://issuer.example/jwks", + }, + }, + { + name: "symmetric algorithm", + provider: authkitoidc.Provider{ + Issuer: "https://issuer.example", + Audiences: []string{testAudience}, + JWKSURL: "https://issuer.example/jwks", + SupportedSigningAlgorithms: []string{"HS256"}, + }, + }, + { + name: "unknown algorithm", + provider: authkitoidc.Provider{ + Issuer: "https://issuer.example", + Audiences: []string{testAudience}, + JWKSURL: "https://issuer.example/jwks", + SupportedSigningAlgorithms: []string{"unknown"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.provider.Validate() + + require.Error(t, err) + }) + } +} diff --git a/proof/oidc/source_test.go b/proof/oidc/source_test.go new file mode 100644 index 0000000..7d14eb4 --- /dev/null +++ b/proof/oidc/source_test.go @@ -0,0 +1,53 @@ +package oidc_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + authkitoidc "github.com/meigma/authkit/proof/oidc" +) + +func TestStaticProviderSourceFindsProvidersByIssuer(t *testing.T) { + source, err := authkitoidc.NewStaticProviderSource(authkitoidc.Provider{ + Issuer: "https://issuer.example", + Audiences: []string{testAudience}, + JWKSURL: "https://issuer.example/jwks", + SupportedSigningAlgorithms: []string{"RS256"}, + }) + require.NoError(t, err) + + provider, err := source.FindProvider(context.Background(), "https://issuer.example") + require.NoError(t, err) + assert.Equal(t, "https://issuer.example", provider.Issuer) + + provider.Audiences[0] = "mutated" + provider, err = source.FindProvider(context.Background(), "https://issuer.example") + require.NoError(t, err) + assert.Equal(t, []string{testAudience}, provider.Audiences) +} + +func TestStaticProviderSourceRejectsDuplicateIssuers(t *testing.T) { + provider := authkitoidc.Provider{ + Issuer: "https://issuer.example", + Audiences: []string{testAudience}, + JWKSURL: "https://issuer.example/jwks", + } + + source, err := authkitoidc.NewStaticProviderSource(provider, provider) + + require.Error(t, err) + assert.Nil(t, source) +} + +func TestStaticProviderSourceReturnsProviderNotFound(t *testing.T) { + source, err := authkitoidc.NewStaticProviderSource() + require.NoError(t, err) + + provider, err := source.FindProvider(context.Background(), "https://issuer.example") + + require.ErrorIs(t, err, authkitoidc.ErrProviderNotFound) + assert.Empty(t, provider) +} diff --git a/proof/oidc/verifier_test.go b/proof/oidc/verifier_test.go index 62371e7..6461ba5 100644 --- a/proof/oidc/verifier_test.go +++ b/proof/oidc/verifier_test.go @@ -2,18 +2,11 @@ package oidc_test import ( "context" - "crypto/rand" - "crypto/rsa" - "encoding/json" "errors" - "net/http" - "net/http/httptest" "testing" "time" - "github.com/lestrrat-go/jwx/v3/jwa" "github.com/lestrrat-go/jwx/v3/jwk" - "github.com/lestrrat-go/jwx/v3/jwt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -21,11 +14,6 @@ import ( authkitoidc "github.com/meigma/authkit/proof/oidc" ) -const ( - testAudience = "authkit-api" - testSubject = "user-123" -) - func TestNewVerifierValidatesDependencies(t *testing.T) { verifier, err := authkitoidc.NewVerifier(nil) @@ -33,127 +21,6 @@ func TestNewVerifierValidatesDependencies(t *testing.T) { assert.Nil(t, verifier) } -func TestProviderValidation(t *testing.T) { - tests := []struct { - name string - provider authkitoidc.Provider - }{ - { - name: "missing issuer", - provider: authkitoidc.Provider{ - Audiences: []string{testAudience}, - JWKSURL: "https://issuer.example/jwks", - }, - }, - { - name: "missing audience", - provider: authkitoidc.Provider{ - Issuer: "https://issuer.example", - JWKSURL: "https://issuer.example/jwks", - }, - }, - { - name: "missing JWKS URL", - provider: authkitoidc.Provider{ - Issuer: "https://issuer.example", - Audiences: []string{testAudience}, - }, - }, - { - name: "insecure issuer", - provider: authkitoidc.Provider{ - Issuer: "http://issuer.example", - Audiences: []string{testAudience}, - JWKSURL: "https://issuer.example/jwks", - }, - }, - { - name: "relative issuer", - provider: authkitoidc.Provider{ - Issuer: "/issuer", - Audiences: []string{testAudience}, - JWKSURL: "https://issuer.example/jwks", - }, - }, - { - name: "insecure JWKS URL", - provider: authkitoidc.Provider{ - Issuer: "https://issuer.example", - Audiences: []string{testAudience}, - JWKSURL: "http://issuer.example/jwks", - }, - }, - { - name: "symmetric algorithm", - provider: authkitoidc.Provider{ - Issuer: "https://issuer.example", - Audiences: []string{testAudience}, - JWKSURL: "https://issuer.example/jwks", - SupportedSigningAlgorithms: []string{"HS256"}, - }, - }, - { - name: "unknown algorithm", - provider: authkitoidc.Provider{ - Issuer: "https://issuer.example", - Audiences: []string{testAudience}, - JWKSURL: "https://issuer.example/jwks", - SupportedSigningAlgorithms: []string{"unknown"}, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.provider.Validate() - - require.Error(t, err) - }) - } -} - -func TestStaticProviderSourceFindsProvidersByIssuer(t *testing.T) { - source, err := authkitoidc.NewStaticProviderSource(authkitoidc.Provider{ - Issuer: "https://issuer.example", - Audiences: []string{testAudience}, - JWKSURL: "https://issuer.example/jwks", - SupportedSigningAlgorithms: []string{"RS256"}, - }) - require.NoError(t, err) - - provider, err := source.FindProvider(context.Background(), "https://issuer.example") - require.NoError(t, err) - assert.Equal(t, "https://issuer.example", provider.Issuer) - - provider.Audiences[0] = "mutated" - provider, err = source.FindProvider(context.Background(), "https://issuer.example") - require.NoError(t, err) - assert.Equal(t, []string{testAudience}, provider.Audiences) -} - -func TestStaticProviderSourceRejectsDuplicateIssuers(t *testing.T) { - provider := authkitoidc.Provider{ - Issuer: "https://issuer.example", - Audiences: []string{testAudience}, - JWKSURL: "https://issuer.example/jwks", - } - - source, err := authkitoidc.NewStaticProviderSource(provider, provider) - - require.Error(t, err) - assert.Nil(t, source) -} - -func TestStaticProviderSourceReturnsProviderNotFound(t *testing.T) { - source, err := authkitoidc.NewStaticProviderSource() - require.NoError(t, err) - - provider, err := source.FindProvider(context.Background(), "https://issuer.example") - - require.ErrorIs(t, err, authkitoidc.ErrProviderNotFound) - assert.Empty(t, provider) -} - func TestVerifierVerifiesValidJWTBearerToken(t *testing.T) { now := fixedTime() issuer := newTestIssuer(t) @@ -470,147 +337,3 @@ func TestVerifierReturnsOperationalErrorsForProviderAndJWKSSourceFailures(t *tes require.NotErrorIs(t, err, authkit.ErrUnauthenticated) assert.Empty(t, identity) } - -type failingProviderSource struct { - err error -} - -func (s failingProviderSource) FindProvider(context.Context, string) (authkitoidc.Provider, error) { - return authkitoidc.Provider{}, s.err -} - -type testIssuer struct { - server *httptest.Server - issuer string - jwksURL string - signingKey jwk.Key - publicSet jwk.Set -} - -type tokenRequest struct { - issuer string - subject string - audiences []string - expiresAt time.Time - notBefore *time.Time - jwtID string - claims map[string]any -} - -func newTestIssuer(t *testing.T) *testIssuer { - t.Helper() - - return newTestIssuerWithPublicKey(t, nil) -} - -func newTestIssuerWithPublicKey(t *testing.T, configure func(*testing.T, jwk.Key)) *testIssuer { - t.Helper() - - rawKey, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - signingKey, err := jwk.Import(rawKey) - require.NoError(t, err) - require.NoError(t, signingKey.Set(jwk.KeyIDKey, "test-key")) - require.NoError(t, signingKey.Set(jwk.AlgorithmKey, jwa.RS256())) - - privateSet := jwk.NewSet() - require.NoError(t, privateSet.AddKey(signingKey)) - publicSet, err := jwk.PublicSetOf(privateSet) - require.NoError(t, err) - if configure != nil { - publicKey, ok := publicSet.Key(0) - require.True(t, ok) - configure(t, publicKey) - } - - issuer := &testIssuer{ - signingKey: signingKey, - publicSet: publicSet, - } - mux := http.NewServeMux() - mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(issuer.publicSet); err != nil { - t.Errorf("encode JWKS: %v", err) - } - }) - mux.HandleFunc("/unavailable", func(w http.ResponseWriter, _ *http.Request) { - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - }) - issuer.server = httptest.NewTLSServer(mux) - t.Cleanup(issuer.server.Close) - issuer.issuer = issuer.server.URL - issuer.jwksURL = issuer.server.URL + "/jwks" - - return issuer -} - -func (i *testIssuer) provider() authkitoidc.Provider { - return authkitoidc.Provider{ - Issuer: i.issuer, - Audiences: []string{testAudience}, - JWKSURL: i.jwksURL, - } -} - -func (i *testIssuer) verifier(t *testing.T, opts ...authkitoidc.Option) *authkitoidc.Verifier { - t.Helper() - - opts = append([]authkitoidc.Option{authkitoidc.WithHTTPClient(i.server.Client())}, opts...) - - return newVerifier(t, i.provider(), opts...) -} - -func (i *testIssuer) sign(t *testing.T, req tokenRequest) string { - t.Helper() - - issuer := req.issuer - if issuer == "" { - issuer = i.issuer - } - builder := jwt.NewBuilder(). - Issuer(issuer). - Audience(req.audiences). - IssuedAt(fixedTime().Add(-time.Minute)) - if req.subject != "" { - builder.Subject(req.subject) - } - if !req.expiresAt.IsZero() { - builder.Expiration(req.expiresAt) - } - if req.notBefore != nil { - builder.NotBefore(*req.notBefore) - } - if req.jwtID != "" { - builder.JwtID(req.jwtID) - } - for name, value := range req.claims { - builder.Claim(name, value) - } - - token, err := builder.Build() - require.NoError(t, err) - signed, err := jwt.Sign(token, jwt.WithKey(jwa.RS256(), i.signingKey)) - require.NoError(t, err) - - return string(signed) -} - -func newVerifier( - t *testing.T, - provider authkitoidc.Provider, - opts ...authkitoidc.Option, -) *authkitoidc.Verifier { - t.Helper() - - source, err := authkitoidc.NewStaticProviderSource(provider) - require.NoError(t, err) - verifier, err := authkitoidc.NewVerifier(source, opts...) - require.NoError(t, err) - - return verifier -} - -func fixedTime() time.Time { - return time.Date(2026, 5, 8, 12, 0, 0, 0, time.UTC) -} From 5e687a9e2ed0b85901b0424645fd5653d5de67c0 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 17:59:22 -0700 Subject: [PATCH 4/4] refactor(proof/oidc): compile-time port assertion for StaticProviderSource Make StaticProviderSource's implementation of ProviderSource a compile-time check so an accidental method signature drift breaks the build rather than a downstream caller. Store-side ProviderSource and ProviderTrustStore assertions already live in store/memory/oidc_test.go and store/postgres/oidc_test.go. Co-Authored-By: Claude Opus 4.7 (1M context) --- proof/oidc/source.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/proof/oidc/source.go b/proof/oidc/source.go index ca764e6..f400403 100644 --- a/proof/oidc/source.go +++ b/proof/oidc/source.go @@ -27,6 +27,8 @@ type StaticProviderSource struct { providers map[string]Provider } +var _ ProviderSource = (*StaticProviderSource)(nil) + // NewStaticProviderSource constructs a static provider source. func NewStaticProviderSource(providers ...Provider) (*StaticProviderSource, error) { source := &StaticProviderSource{