Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ require (
github.com/gravitational/roundtrip v1.0.2
github.com/gravitational/teleport/api v0.0.0
github.com/gravitational/trace v1.5.1
github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Some library usage notes:

  • gregjones/httpcache is deprecated and archived, however...
  • We already include this library via the Kubernetes client library
  • It otherwise appears to be fit for purpose, and no other similarly mature libraries exist (bartventer/httpcache exists but is much less mature)
  • It is MIT licensed

I think we can switch to an alternative if/when we find a compelling reason to do so - it should just mean swapping out the implementation we return in NewCachingHTTPClient().

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to use a third party library for caching jwks?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I mean, we certainly could build everything ourselves, but we'd need to rewrite a meaningful portion of zitadel/oidc to support caching natively, and then still build our own RFC 9111-compliant HTTP client anyway since the only way for providers to provide cache hints is through standard HTTP caching headers. We could disregard cache hints and just set a modest value, but then we're still stuck making fairly sweeping changes to our OIDC library.

From what I can tell, just using a caching *http.Client achieves fundamentally the same thing with very little effort required on our end. That said, I'd be very happy to hear suggestions if there's simpler alternatives I've missed!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My concern is that the memory cache is unbounded, and entries are only cleared upon retries. Since the cache key is based on the URL, if we ever introduce dynamic fields in the JWKS endpoint-such as a timestamp-or encounter an unusually large number of OIDC tokens, memory usage could grow significantly.

Copy link
Copy Markdown
Contributor Author

@timothyb89 timothyb89 Aug 12, 2025

Choose a reason for hiding this comment

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

Hmm, that's a fair point. We could use a LRU cache backend instead, https://github.com/die-net/lrucache is linked directly from gregjones/httpcache - though similar maintenance caveats apply - or I could build one off github.com/hashicorp/golang-lru which we already include.

That said, I don't expect many problems here in practice:

  • OIDC endpoints themselves are user specified and cannot change regularly
  • JWKS URIs technically could change but zero providers I'm currently aware of do so.

I do agree that an LRU cache of some sort is much better but this trivial implementation is unlikely to actually grow unbounded in practice. In any case, I'll look into the best path for evicting entries.

github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.1.0
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2
github.com/guptarohit/asciigraph v0.7.3
Expand Down Expand Up @@ -411,7 +412,6 @@ require (
github.com/gorilla/securecookie v1.1.2 // indirect
github.com/gosuri/uitable v0.0.4 // indirect
github.com/grafana/pyroscope-go/godeltaprof v0.1.8 // indirect
github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.1 // indirect
github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect
github.com/hailocab/go-hostpool v0.0.0-20160125115350-e80d13ce29ed // indirect
Expand Down
4 changes: 2 additions & 2 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ func NewServer(cfg *InitConfig, opts ...ServerOption) (as *Server, err error) {
as.k8sJWKSValidator = kubetoken.ValidateTokenWithJWKS
}
if as.k8sOIDCValidator == nil {
as.k8sOIDCValidator = kubetoken.ValidateTokenWithOIDC
as.k8sOIDCValidator = kubetoken.NewKubernetesOIDCTokenValidator()
}

if as.gcpIDTokenValidator == nil {
Expand Down Expand Up @@ -1225,7 +1225,7 @@ type Server struct {
k8sJWKSValidator k8sJWKSValidator
// k8sOIDCValidator allows tokens from Kubernetes to be validated by the
// auth server using a known OIDC endpoint. It can be overridden in tests.
k8sOIDCValidator k8sOIDCValidator
k8sOIDCValidator *kubetoken.KubernetesOIDCTokenValidator

// gcpIDTokenValidator allows ID tokens from GCP to be validated by the auth
// server. It can be overridden for the purpose of tests.
Expand Down
9 changes: 1 addition & 8 deletions lib/auth/join_kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,6 @@ type k8sTokenReviewValidator interface {

type k8sJWKSValidator func(now time.Time, jwksData []byte, clusterName string, token string) (*kubetoken.ValidationResult, error)

type k8sOIDCValidator func(
ctx context.Context,
issuerURL string,
clusterName string,
token string,
) (*kubetoken.ValidationResult, error)

func (a *Server) checkKubernetesJoinRequest(
ctx context.Context,
req *types.RegisterUsingTokenRequest,
Expand Down Expand Up @@ -77,7 +70,7 @@ func (a *Server) checkKubernetesJoinRequest(
return nil, trace.WrapWithMessage(err, "reviewing kubernetes token with static_jwks")
}
case types.KubernetesJoinTypeOIDC:
result, err = a.k8sOIDCValidator(
result, err = a.k8sOIDCValidator.ValidateTokenWithOIDC(
ctx,
token.Spec.Kubernetes.OIDC.Issuer,
clusterName,
Expand Down
20 changes: 18 additions & 2 deletions lib/kube/token/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package token
import (
"context"
"encoding/json"
"net/http"
"slices"
"strings"
"sync"
Expand Down Expand Up @@ -361,16 +362,31 @@ func ValidateTokenWithJWKS(
}, nil
}

// KubernetesOIDCTokenValidator is a utility struct to retain a caching HTTP
// client for use during OIDC validation.
type KubernetesOIDCTokenValidator struct {
client *http.Client
}

// NewKubernetesOIDCTokenValidator returns a token validator populated with a
// caching HTTP client.
func NewKubernetesOIDCTokenValidator() *KubernetesOIDCTokenValidator {
return &KubernetesOIDCTokenValidator{
client: oidc.NewCachingHTTPClient(),
}
}

// ValidateTokenWithJWKS validates a Kubernetes Service Account JWT using an
// OIDC endpoint.
func ValidateTokenWithOIDC(
func (v *KubernetesOIDCTokenValidator) ValidateTokenWithOIDC(
ctx context.Context,
issuerURL string,
clusterName string,
token string,
) (*ValidationResult, error) {
claims, err := oidc.ValidateToken[*OIDCServiceAccountClaims](
claims, err := oidc.ValidateTokenWithClient[*OIDCServiceAccountClaims](
ctx,
v.client,
issuerURL,
clusterName,
token,
Expand Down
5 changes: 4 additions & 1 deletion lib/kube/token/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -956,7 +956,10 @@ func TestValidateTokenWithOIDC(t *testing.T) {

issuer := fmt.Sprintf("http://%s", idp.server.Listener.Addr().String())

result, err := ValidateTokenWithOIDC(ctx, issuer, tt.audience, tt.token)
v := KubernetesOIDCTokenValidator{
client: http.DefaultClient,
}
result, err := v.ValidateTokenWithOIDC(ctx, issuer, tt.audience, tt.token)
tt.assertError(t, err)

require.Empty(t, cmp.Diff(
Expand Down
39 changes: 37 additions & 2 deletions lib/oidc/token_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ package oidc

import (
"context"
"net/http"
"time"

"github.com/gravitational/trace"
"github.com/gregjones/httpcache"
"github.com/zitadel/oidc/v3/pkg/client"
"github.com/zitadel/oidc/v3/pkg/client/rp"
"github.com/zitadel/oidc/v3/pkg/oidc"
Expand All @@ -40,6 +42,28 @@ func ValidateToken[C oidc.Claims](
audience string,
token string,
opts ...rp.VerifierOption,
) (C, error) {
return ValidateTokenWithClient[C](
ctx,
otelhttp.DefaultClient,
issuerURL,
audience,
token,
opts...,
)
}

// ValidateTokenWithClient validates an OIDC token against a generic claim type,
// but accepts a custom HTTP client. This custom client can be used in tandem
// with the client returned by `NewCachingHTTPClient()` to cache OpenID and JWKS
// responses.
func ValidateTokenWithClient[C oidc.Claims](
ctx context.Context,
httpClient *http.Client,
issuerURL string,
audience string,
token string,
opts ...rp.VerifierOption,
) (C, error) {
timeoutCtx, cancel := context.WithTimeout(ctx, providerTimeout)
defer cancel()
Expand All @@ -49,14 +73,14 @@ func ValidateToken[C oidc.Claims](
// TODO(noah): It'd be nice to cache the OIDC discovery document fairly
// aggressively across join tokens since this isn't going to change very
// regularly.
dc, err := client.Discover(timeoutCtx, issuerURL, otelhttp.DefaultClient)
dc, err := client.Discover(timeoutCtx, issuerURL, httpClient)
if err != nil {
return nilClaims, trace.Wrap(err, "discovering oidc document")
}

// TODO(noah): Ideally we'd cache the remote keyset across joins/join tokens
// based on the issuer.
ks := rp.NewRemoteKeySet(otelhttp.DefaultClient, dc.JwksURI)
ks := rp.NewRemoteKeySet(httpClient, dc.JwksURI)
verifier := rp.NewIDTokenVerifier(issuerURL, audience, ks, opts...)
// TODO(noah): It'd be ideal if we could extend the verifier to use an
// injected "now" time.
Expand All @@ -68,3 +92,14 @@ func ValidateToken[C oidc.Claims](

return claims, nil
}

// NewCachingHTTPClient constructs and returns a caching HTTP client. Similar to
// the default client used in `ValidateToken()`, this uses an underlying
// `otelhttp` transport.
// Note that this client should be retained and reused to benefit from caching.
func NewCachingHTTPClient() *http.Client {
transport := httpcache.NewMemoryCacheTransport()
transport.Transport = otelhttp.NewTransport(http.DefaultTransport)

return transport.Client()
}
Loading