diff --git a/internal/controller/authenticator/authenticator.go b/internal/controller/authenticator/authenticator.go new file mode 100644 index 00000000..16075ab5 --- /dev/null +++ b/internal/controller/authenticator/authenticator.go @@ -0,0 +1,21 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package authenticator contains helper code for dealing with *Authenticator CRDs. +package authenticator + +import ( + "encoding/base64" + + auth1alpha1 "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1" +) + +// CABundle returns a PEM-encoded CA bundle from the provided spec. If the provided spec is nil, a +// nil CA bundle will be returned. If the provided spec contains a CA bundle that is not properly +// encoded, an error will be returned. +func CABundle(spec *auth1alpha1.TLSSpec) ([]byte, error) { + if spec == nil { + return nil, nil + } + return base64.StdEncoding.DecodeString(spec.CertificateAuthorityData) +} diff --git a/internal/controller/authenticator/authncache/cache.go b/internal/controller/authenticator/authncache/cache.go index 00de2e1e..5e8922e3 100644 --- a/internal/controller/authenticator/authncache/cache.go +++ b/internal/controller/authenticator/authncache/cache.go @@ -12,8 +12,10 @@ import ( "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/klog/v2" loginapi "go.pinniped.dev/generated/1.19/apis/concierge/login" + "go.pinniped.dev/internal/plog" ) var ( @@ -93,6 +95,12 @@ func (c *Cache) AuthenticateTokenCredentialRequest(ctx context.Context, req *log val := c.Get(key) if val == nil { + plog.Debug( + "authenticator does not exist", + "authenticator", klog.KRef(key.Namespace, key.Name), + "kind", key.Kind, + "apiGroup", key.APIGroup, + ) return nil, ErrNoSuchAuthenticator } diff --git a/internal/controller/authenticator/cachecleaner/cachecleaner.go b/internal/controller/authenticator/cachecleaner/cachecleaner.go new file mode 100644 index 00000000..ab762536 --- /dev/null +++ b/internal/controller/authenticator/cachecleaner/cachecleaner.go @@ -0,0 +1,119 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package cachecleaner implements a controller for garbage collecting authenticators from an authenticator cache. +package cachecleaner + +import ( + "fmt" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/klog/v2" + + auth1alpha1 "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1" + authinformers "go.pinniped.dev/generated/1.19/client/concierge/informers/externalversions/authentication/v1alpha1" + pinnipedcontroller "go.pinniped.dev/internal/controller" + "go.pinniped.dev/internal/controller/authenticator/authncache" + "go.pinniped.dev/internal/controllerlib" +) + +// closable is used to detect when a cache value has a Close() method on it. We use this to +// determine if we should call the Close() method on a cache value upon deleting it from the cache. +type closable interface { + Close() +} + +// New instantiates a new controllerlib.Controller which will garbage collect authenticators from the provided Cache. +func New( + cache *authncache.Cache, + webhooks authinformers.WebhookAuthenticatorInformer, + jwtAuthenticators authinformers.JWTAuthenticatorInformer, + log logr.Logger, +) controllerlib.Controller { + return controllerlib.New( + controllerlib.Config{ + Name: "cachecleaner-controller", + Syncer: &controller{ + cache: cache, + webhooks: webhooks, + jwtAuthenticators: jwtAuthenticators, + log: log.WithName("cachecleaner-controller"), + }, + }, + controllerlib.WithInformer( + webhooks, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + controllerlib.WithInformer( + jwtAuthenticators, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + ) +} + +type controller struct { + cache *authncache.Cache + webhooks authinformers.WebhookAuthenticatorInformer + jwtAuthenticators authinformers.JWTAuthenticatorInformer + log logr.Logger +} + +// Sync implements controllerlib.Syncer. +func (c *controller) Sync(_ controllerlib.Context) error { + webhooks, err := c.webhooks.Lister().List(labels.Everything()) + if err != nil { + return fmt.Errorf("failed to list WebhookAuthenticators: %w", err) + } + + jwtAuthenticators, err := c.jwtAuthenticators.Lister().List(labels.Everything()) + if err != nil { + return fmt.Errorf("failed to list JWTAuthenticators: %w", err) + } + + // Index the current authenticators by cache key. + authenticatorSet := map[authncache.Key]bool{} + for _, webhook := range webhooks { + key := authncache.Key{ + Namespace: webhook.Namespace, + Name: webhook.Name, + Kind: "WebhookAuthenticator", + APIGroup: auth1alpha1.SchemeGroupVersion.Group, + } + authenticatorSet[key] = true + } + for _, jwtAuthenticator := range jwtAuthenticators { + key := authncache.Key{ + Namespace: jwtAuthenticator.Namespace, + Name: jwtAuthenticator.Name, + Kind: "JWTAuthenticator", + APIGroup: auth1alpha1.SchemeGroupVersion.Group, + } + authenticatorSet[key] = true + } + + // Delete any entries from the cache which are no longer in the cluster. + for _, key := range c.cache.Keys() { + if key.APIGroup != auth1alpha1.SchemeGroupVersion.Group || (key.Kind != "WebhookAuthenticator" && key.Kind != "JWTAuthenticator") { + continue + } + if _, exists := authenticatorSet[key]; !exists { + c.log.WithValues( + "authenticator", + klog.KRef(key.Namespace, key.Name), + "kind", + key.Kind, + ).Info("deleting authenticator from cache") + + value := c.cache.Get(key) + if closable, ok := value.(closable); ok { + closable.Close() + } + + c.cache.Delete(key) + } + } + return nil +} diff --git a/internal/controller/authenticator/cachecleaner/cachecleaner_test.go b/internal/controller/authenticator/cachecleaner/cachecleaner_test.go new file mode 100644 index 00000000..9e5cd147 --- /dev/null +++ b/internal/controller/authenticator/cachecleaner/cachecleaner_test.go @@ -0,0 +1,207 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cachecleaner + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + authv1alpha "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1" + pinnipedfake "go.pinniped.dev/generated/1.19/client/concierge/clientset/versioned/fake" + pinnipedinformers "go.pinniped.dev/generated/1.19/client/concierge/informers/externalversions" + "go.pinniped.dev/internal/controller/authenticator/authncache" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/testutil/testlogger" +) + +func TestController(t *testing.T) { + t.Parallel() + + testWebhookKey1 := authncache.Key{ + APIGroup: "authentication.concierge.pinniped.dev", + Kind: "WebhookAuthenticator", + Namespace: "test-namespace", + Name: "test-webhook-name-one", + } + testWebhookKey2 := authncache.Key{ + APIGroup: "authentication.concierge.pinniped.dev", + Kind: "WebhookAuthenticator", + Namespace: "test-namespace", + Name: "test-webhook-name-two", + } + testJWTAuthenticatorKey1 := authncache.Key{ + APIGroup: "authentication.concierge.pinniped.dev", + Kind: "JWTAuthenticator", + Namespace: "test-namespace", + Name: "test-jwt-authenticator-name-one", + } + testJWTAuthenticatorKey2 := authncache.Key{ + APIGroup: "authentication.concierge.pinniped.dev", + Kind: "JWTAuthenticator", + Namespace: "test-namespace", + Name: "test-jwt-authenticator-name-two", + } + testKeyUnknownType := authncache.Key{ + APIGroup: "authentication.concierge.pinniped.dev", + Kind: "SomeOtherAuthenticator", + Namespace: "test-namespace", + Name: "test-name-one", + } + + tests := []struct { + name string + objects []runtime.Object + initialCache map[authncache.Key]authncache.Value + wantErr string + wantLogs []string + wantCacheKeys []authncache.Key + }{ + { + name: "no change", + initialCache: map[authncache.Key]authncache.Value{ + testWebhookKey1: nil, + testJWTAuthenticatorKey1: nil, + }, + objects: []runtime.Object{ + &authv1alpha.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testWebhookKey1.Namespace, + Name: testWebhookKey1.Name, + }, + }, + &authv1alpha.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testJWTAuthenticatorKey1.Namespace, + Name: testJWTAuthenticatorKey1.Name, + }, + }, + }, + wantCacheKeys: []authncache.Key{testWebhookKey1, testJWTAuthenticatorKey1}, + }, + { + name: "authenticators not yet added", + initialCache: nil, + objects: []runtime.Object{ + &authv1alpha.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testWebhookKey1.Namespace, + Name: testWebhookKey1.Name, + }, + }, + &authv1alpha.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testWebhookKey2.Namespace, + Name: testWebhookKey2.Name, + }, + }, + &authv1alpha.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testJWTAuthenticatorKey1.Namespace, + Name: testJWTAuthenticatorKey1.Name, + }, + }, + &authv1alpha.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testJWTAuthenticatorKey2.Namespace, + Name: testJWTAuthenticatorKey2.Name, + }, + }, + }, + wantCacheKeys: []authncache.Key{}, + }, + { + name: "successful cleanup", + initialCache: map[authncache.Key]authncache.Value{ + testWebhookKey1: nil, + testWebhookKey2: nil, + testJWTAuthenticatorKey1: newClosableCacheValue(t, "closable1", 0), + testJWTAuthenticatorKey2: newClosableCacheValue(t, "closable2", 1), + testKeyUnknownType: nil, + }, + objects: []runtime.Object{ + &authv1alpha.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testWebhookKey1.Namespace, + Name: testWebhookKey1.Name, + }, + }, + &authv1alpha.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testJWTAuthenticatorKey1.Namespace, + Name: testJWTAuthenticatorKey1.Name, + }, + }, + }, + wantLogs: []string{ + `cachecleaner-controller "level"=0 "msg"="deleting authenticator from cache" "authenticator"={"name":"test-jwt-authenticator-name-two","namespace":"test-namespace"} "kind"="JWTAuthenticator"`, + `cachecleaner-controller "level"=0 "msg"="deleting authenticator from cache" "authenticator"={"name":"test-webhook-name-two","namespace":"test-namespace"} "kind"="WebhookAuthenticator"`, + }, + wantCacheKeys: []authncache.Key{testWebhookKey1, testJWTAuthenticatorKey1, testKeyUnknownType}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + // When we have t.Parallel() here, this test blocks pretty consistently...y tho? + + fakeClient := pinnipedfake.NewSimpleClientset(tt.objects...) + informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0) + cache := authncache.New() + for k, v := range tt.initialCache { + cache.Store(k, v) + } + testLog := testlogger.New(t) + + webhooks := informers.Authentication().V1alpha1().WebhookAuthenticators() + jwtAuthenticators := informers.Authentication().V1alpha1().JWTAuthenticators() + controller := New(cache, webhooks, jwtAuthenticators, testLog) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + informers.Start(ctx.Done()) + informers.WaitForCacheSync(ctx.Done()) + controllerlib.TestRunSynchronously(t, controller) + + syncCtx := controllerlib.Context{ + Context: ctx, + Key: controllerlib.Key{ + Namespace: "test-namespace", + Name: "test-webhook-name-one", + }, + } + + if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.ElementsMatch(t, tt.wantLogs, testLog.Lines()) + require.ElementsMatch(t, tt.wantCacheKeys, cache.Keys()) + }) + } +} + +func newClosableCacheValue(t *testing.T, name string, wantCloses int) authncache.Value { + t.Helper() + c := &closableCacheValue{} + t.Cleanup(func() { + require.Equalf(t, wantCloses, c.closeCallCount, "expected %s.Close() to be called %d times", name, wantCloses) + }) + return c +} + +type closableCacheValue struct { + authncache.Value + closeCallCount int +} + +func (c *closableCacheValue) Close() { + c.closeCallCount++ +} diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go new file mode 100644 index 00000000..3028fbf8 --- /dev/null +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -0,0 +1,136 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package jwtcachefiller implements a controller for filling an authncache.Cache with each +// added/updated JWTAuthenticator. +package jwtcachefiller + +import ( + "fmt" + "io/ioutil" + "os" + + "github.com/go-logr/logr" + "gopkg.in/square/go-jose.v2" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apiserver/plugin/pkg/authenticator/token/oidc" + "k8s.io/klog/v2" + + auth1alpha1 "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1" + authinformers "go.pinniped.dev/generated/1.19/client/concierge/informers/externalversions/authentication/v1alpha1" + pinnipedcontroller "go.pinniped.dev/internal/controller" + "go.pinniped.dev/internal/controller/authenticator" + "go.pinniped.dev/internal/controller/authenticator/authncache" + "go.pinniped.dev/internal/controllerlib" +) + +// These default values come from the way that the Supervisor issues and signs tokens. We make these +// the defaults for a JWTAuthenticator so that they can easily integrate with the Supervisor. +const ( + defaultUsernameClaim = "sub" + defaultGroupsClaim = "groups" +) + +// defaultSupportedSigningAlgos returns the default signing algos that this JWTAuthenticator +// supports (i.e., if none are supplied by the user). +func defaultSupportedSigningAlgos() []string { + return []string{ + // RS256 is recommended by the OIDC spec and required, in some capacity. Since we want the + // JWTAuthenticator to be able to support many OIDC ID tokens out of the box, we include this + // algorithm by default. + string(jose.RS256), + // ES256 is what the Supervisor does, by default. We want integration with the JWTAuthenticator + // to be as seamless as possible, so we include this algorithm by default. + string(jose.ES256), + } +} + +// New instantiates a new controllerlib.Controller which will populate the provided authncache.Cache. +func New( + cache *authncache.Cache, + jwtAuthenticators authinformers.JWTAuthenticatorInformer, + log logr.Logger, +) controllerlib.Controller { + return controllerlib.New( + controllerlib.Config{ + Name: "jwtcachefiller-controller", + Syncer: &controller{ + cache: cache, + jwtAuthenticators: jwtAuthenticators, + log: log.WithName("jwtcachefiller-controller"), + }, + }, + controllerlib.WithInformer( + jwtAuthenticators, + pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct + controllerlib.InformerOption{}, + ), + ) +} + +type controller struct { + cache *authncache.Cache + jwtAuthenticators authinformers.JWTAuthenticatorInformer + log logr.Logger +} + +// Sync implements controllerlib.Syncer. +func (c *controller) Sync(ctx controllerlib.Context) error { + obj, err := c.jwtAuthenticators.Lister().JWTAuthenticators(ctx.Key.Namespace).Get(ctx.Key.Name) + if err != nil && errors.IsNotFound(err) { + c.log.Info("Sync() found that the JWTAuthenticator does not exist yet or was deleted") + return nil + } + if err != nil { + return fmt.Errorf("failed to get JWTAuthenticator %s/%s: %w", ctx.Key.Namespace, ctx.Key.Name, err) + } + + jwtAuthenticator, err := newJWTAuthenticator(&obj.Spec) + if err != nil { + return fmt.Errorf("failed to build jwt authenticator: %w", err) + } + + c.cache.Store(authncache.Key{ + APIGroup: auth1alpha1.GroupName, + Kind: "JWTAuthenticator", + Namespace: ctx.Key.Namespace, + Name: ctx.Key.Name, + }, jwtAuthenticator) + c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer).Info("added new jwt authenticator") + return nil +} + +// newJWTAuthenticator creates a jwt authenticator from the provided spec. +func newJWTAuthenticator(spec *auth1alpha1.JWTAuthenticatorSpec) (*oidc.Authenticator, error) { + caBundle, err := authenticator.CABundle(spec.TLS) + if err != nil { + return nil, fmt.Errorf("invalid TLS configuration: %w", err) + } + + var caFile string + if caBundle != nil { + temp, err := ioutil.TempFile("", "pinniped-jwkauthenticator-cafile-*") + if err != nil { + return nil, fmt.Errorf("unable to create temporary file: %w", err) + } + + // We can safely remove the temp file at the end of this function since oidc.New() reads the + // provided CA file and then forgets about it. + defer func() { _ = os.Remove(temp.Name()) }() + + if _, err := temp.Write(caBundle); err != nil { + return nil, fmt.Errorf("cannot write CA file: %w", err) + } + + caFile = temp.Name() + } + + return oidc.New(oidc.Options{ + IssuerURL: spec.Issuer, + ClientID: spec.Audience, + UsernameClaim: defaultUsernameClaim, + GroupsClaim: defaultGroupsClaim, + SupportedSigningAlgs: defaultSupportedSigningAlgos(), + CAFile: caFile, + }) +} diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go new file mode 100644 index 00000000..c5d453f2 --- /dev/null +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -0,0 +1,438 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package jwtcachefiller + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "encoding/base64" + "encoding/json" + "encoding/pem" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "gopkg.in/square/go-jose.v2/jwt" + + "github.com/stretchr/testify/require" + "gopkg.in/square/go-jose.v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/authentication/authenticator" + "k8s.io/apiserver/pkg/authentication/user" + + auth1alpha1 "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1" + pinnipedfake "go.pinniped.dev/generated/1.19/client/concierge/clientset/versioned/fake" + pinnipedinformers "go.pinniped.dev/generated/1.19/client/concierge/informers/externalversions" + "go.pinniped.dev/internal/controller/authenticator/authncache" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/testutil/testlogger" +) + +func TestController(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + syncKey controllerlib.Key + jwtAuthenticators []runtime.Object + wantErr string + wantLogs []string + wantCacheEntries int + }{ + { + name: "not found", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="Sync() found that the JWTAuthenticator does not exist yet or was deleted"`, + }, + }, + { + name: "valid jwt authenticator with CA", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: auth1alpha1.JWTAuthenticatorSpec{ + Issuer: "https://some-issuer.com", + Audience: "some-audience", + TLS: &auth1alpha1.TLSSpec{CertificateAuthorityData: "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVVENDQWptZ0F3SUJBZ0lWQUpzNStTbVRtaTJXeUI0bGJJRXBXaUs5a1RkUE1BMEdDU3FHU0liM0RRRUIKQ3dVQU1COHhDekFKQmdOVkJBWVRBbFZUTVJBd0RnWURWUVFLREFkUWFYWnZkR0ZzTUI0WERUSXdNRFV3TkRFMgpNamMxT0ZvWERUSTBNRFV3TlRFMk1qYzFPRm93SHpFTE1Ba0dBMVVFQmhNQ1ZWTXhFREFPQmdOVkJBb01CMUJwCmRtOTBZV3d3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRRERZWmZvWGR4Z2NXTEMKZEJtbHB5a0tBaG9JMlBuUWtsVFNXMno1cGcwaXJjOGFRL1E3MXZzMTRZYStmdWtFTGlvOTRZYWw4R01DdVFrbApMZ3AvUEE5N1VYelhQNDBpK25iNXcwRGpwWWd2dU9KQXJXMno2MFRnWE5NSFh3VHk4ME1SZEhpUFVWZ0VZd0JpCmtkNThzdEFVS1Y1MnBQTU1reTJjNy9BcFhJNmRXR2xjalUvaFBsNmtpRzZ5dEw2REtGYjJQRWV3MmdJM3pHZ2IKOFVVbnA1V05DZDd2WjNVY0ZHNXlsZEd3aGc3cnZ4U1ZLWi9WOEhCMGJmbjlxamlrSVcxWFM4dzdpUUNlQmdQMApYZWhKZmVITlZJaTJtZlczNlVQbWpMdnVKaGpqNDIrdFBQWndvdDkzdWtlcEgvbWpHcFJEVm9wamJyWGlpTUYrCkYxdnlPNGMxQWdNQkFBR2pnWU13Z1lBd0hRWURWUjBPQkJZRUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1IKTUI4R0ExVWRJd1FZTUJhQUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1JNQjBHQTFVZEpRUVdNQlFHQ0NzRwpBUVVGQndNQ0JnZ3JCZ0VGQlFjREFUQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BNEdBMVVkRHdFQi93UUVBd0lCCkJqQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFYbEh4M2tIMDZwY2NDTDlEVE5qTnBCYnlVSytGd2R6T2IwWFYKcmpNaGtxdHVmdEpUUnR5T3hKZ0ZKNXhUR3pCdEtKamcrVU1pczBOV0t0VDBNWThVMU45U2c5SDl0RFpHRHBjVQpxMlVRU0Y4dXRQMVR3dnJIUzIrdzB2MUoxdHgrTEFiU0lmWmJCV0xXQ21EODUzRlVoWlFZekkvYXpFM28vd0p1CmlPUklMdUpNUk5vNlBXY3VLZmRFVkhaS1RTWnk3a25FcHNidGtsN3EwRE91eUFWdG9HVnlkb3VUR0FOdFhXK2YKczNUSTJjKzErZXg3L2RZOEJGQTFzNWFUOG5vZnU3T1RTTzdiS1kzSkRBUHZOeFQzKzVZUXJwNGR1Nmh0YUFMbAppOHNaRkhidmxpd2EzdlhxL3p1Y2JEaHEzQzBhZnAzV2ZwRGxwSlpvLy9QUUFKaTZLQT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"}, + }, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="https://some-issuer.com" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + }, + { + name: "valid jwt authenticator without CA", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: auth1alpha1.JWTAuthenticatorSpec{ + Issuer: "https://some-issuer.com", + Audience: "some-audience", + }, + }, + }, + wantLogs: []string{ + `jwtcachefiller-controller "level"=0 "msg"="added new jwt authenticator" "issuer"="https://some-issuer.com" "jwtAuthenticator"={"name":"test-name","namespace":"test-namespace"}`, + }, + wantCacheEntries: 1, + }, + { + name: "invalid jwt authenticator CA", + syncKey: controllerlib.Key{Namespace: "test-namespace", Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-name", + }, + Spec: auth1alpha1.JWTAuthenticatorSpec{ + Issuer: "https://some-issuer.com", + Audience: "some-audience", + TLS: &auth1alpha1.TLSSpec{CertificateAuthorityData: "not base64-encoded"}, + }, + }, + }, + wantErr: "failed to build jwt authenticator: invalid TLS configuration: illegal base64 data at input byte 3", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fakeClient := pinnipedfake.NewSimpleClientset(tt.jwtAuthenticators...) + informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0) + cache := authncache.New() + testLog := testlogger.New(t) + + controller := New(cache, informers.Authentication().V1alpha1().JWTAuthenticators(), testLog) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + informers.Start(ctx.Done()) + controllerlib.TestRunSynchronously(t, controller) + + syncCtx := controllerlib.Context{Context: ctx, Key: tt.syncKey} + + if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantLogs, testLog.Lines()) + require.Equal(t, tt.wantCacheEntries, len(cache.Keys())) + }) + } +} + +func TestNewJWTAuthenticator(t *testing.T) { + t.Parallel() + + const ( + goodSubject = "some-subject" + goodAudience = "some-audience" + group0 = "some-group-0" + group1 = "some-group-1" + + goodECSigningKeyID = "some-ec-key-id" + goodRSASigningKeyID = "some-rsa-key-id" + ) + + goodECSigningKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + goodECSigningAlgo := jose.ES256 + require.NoError(t, err) + + goodRSASigningKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + goodRSASigningAlgo := jose.RS256 + + mux := http.NewServeMux() + server := httptest.NewTLSServer(mux) + t.Cleanup(server.Close) + + mux.Handle("/.well-known/openid-configuration", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, err := fmt.Fprintf(w, `{"issuer": "%s", "jwks_uri": "%s"}`, server.URL, server.URL+"/jwks.json") + require.NoError(t, err) + })) + mux.Handle("/jwks.json", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ecJWK := jose.JSONWebKey{ + Key: goodECSigningKey, + KeyID: goodECSigningKeyID, + Algorithm: string(goodECSigningAlgo), + Use: "sig", + } + rsaJWK := jose.JSONWebKey{ + Key: goodRSASigningKey, + KeyID: goodRSASigningKeyID, + Algorithm: string(goodRSASigningAlgo), + Use: "sig", + } + jwks := jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ecJWK.Public(), rsaJWK.Public()}, + } + require.NoError(t, json.NewEncoder(w).Encode(jwks)) + })) + + goodIssuer := server.URL + a, err := newJWTAuthenticator(&auth1alpha1.JWTAuthenticatorSpec{ + Issuer: goodIssuer, + Audience: goodAudience, + TLS: tlsSpecFromTLSConfig(server.TLS), + }) + require.NoError(t, err) + t.Cleanup(a.Close) + + // The implementation of AuthenticateToken() that we use waits 10 seconds after creation to + // perform OIDC discovery. Therefore, the JWTAuthenticator is not functional for the first 10 + // seconds. We sleep for 13 seconds in this unit test to give a little bit of cushion to that 10 + // second delay. + // + // We should get rid of this 10 second delay. See + // https://github.com/vmware-tanzu/pinniped/issues/260. + if testing.Short() { + t.Skip("skipping this test when '-short' flag is passed to avoid necessary 13 second sleep") + } + time.Sleep(time.Second * 13) + + var tests = []struct { + name string + jwtClaims func(wellKnownClaims *jwt.Claims, groups *interface{}) + jwtSignature func(key *interface{}, algo *jose.SignatureAlgorithm, kid *string) + wantResponse *authenticator.Response + wantAuthenticated bool + wantErrorRegexp string + }{ + { + name: "good token without groups and with EC signature", + wantResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: goodSubject, + }, + }, + wantAuthenticated: true, + }, + { + name: "good token without groups and with RSA signature", + jwtSignature: func(key *interface{}, algo *jose.SignatureAlgorithm, kid *string) { + *key = goodRSASigningKey + *algo = goodRSASigningAlgo + *kid = goodRSASigningKeyID + }, + wantResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: goodSubject, + }, + }, + wantAuthenticated: true, + }, + { + name: "good token with groups as array", + jwtClaims: func(_ *jwt.Claims, groups *interface{}) { + *groups = []string{group0, group1} + }, + wantResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: goodSubject, + Groups: []string{group0, group1}, + }, + }, + wantAuthenticated: true, + }, + { + name: "good token with groups as string", + jwtClaims: func(_ *jwt.Claims, groups *interface{}) { + *groups = group0 + }, + wantResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: goodSubject, + Groups: []string{group0}, + }, + }, + wantAuthenticated: true, + }, + { + name: "good token with nbf unset", + jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + claims.NotBefore = nil + }, + wantResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: goodSubject, + }, + }, + wantAuthenticated: true, + }, + { + name: "bad token with groups as map", + jwtClaims: func(_ *jwt.Claims, groups *interface{}) { + *groups = map[string]string{"not an array": "or a string"} + }, + wantErrorRegexp: "oidc: parse groups claim \"groups\": json: cannot unmarshal object into Go value of type string", + }, + { + name: "bad token with wrong issuer", + jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + claims.Issuer = "wrong-issuer" + }, + wantResponse: nil, + wantAuthenticated: false, + }, + { + name: "bad token with no audience", + jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + claims.Audience = nil + }, + wantErrorRegexp: `oidc: verify token: oidc: expected audience "some-audience" got \[\]`, + }, + { + name: "bad token with wrong audience", + jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + claims.Audience = []string{"wrong-audience"} + }, + wantErrorRegexp: `oidc: verify token: oidc: expected audience "some-audience" got \["wrong-audience"\]`, + }, + { + name: "bad token with nbf in the future", + jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + claims.NotBefore = jwt.NewNumericDate(time.Date(3020, 2, 3, 4, 5, 6, 7, time.UTC)) + }, + wantErrorRegexp: `oidc: verify token: oidc: current time .* before the nbf \(not before\) time: 3020-.*`, + }, + { + name: "bad token with exp in past", + jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + claims.Expiry = jwt.NewNumericDate(time.Date(1, 2, 3, 4, 5, 6, 7, time.UTC)) + }, + wantErrorRegexp: `oidc: verify token: oidc: token is expired \(Token Expiry: 0001-02-02 23:09:04 -0456 LMT\)`, + }, + { + name: "bad token without exp", + jwtClaims: func(claims *jwt.Claims, _ *interface{}) { + claims.Expiry = nil + }, + wantErrorRegexp: `oidc: verify token: oidc: token is expired \(Token Expiry: 0001-01-01 00:00:00 \+0000 UTC\)`, + }, + { + name: "signing key is wrong", + jwtSignature: func(key *interface{}, algo *jose.SignatureAlgorithm, kid *string) { + var err error + *key, err = ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + *algo = jose.ES256 + }, + wantErrorRegexp: `oidc: verify token: failed to verify signature: failed to verify id token signature`, + }, + { + name: "signing algo is unsupported", + jwtSignature: func(key *interface{}, algo *jose.SignatureAlgorithm, kid *string) { + var err error + *key, err = ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + require.NoError(t, err) + *algo = jose.ES384 + }, + wantErrorRegexp: `oidc: verify token: oidc: id token signed with unsupported algorithm, expected \["RS256" "ES256"\] got "ES384"`, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + wellKnownClaims := jwt.Claims{ + Issuer: goodIssuer, + Subject: goodSubject, + Audience: []string{goodAudience}, + Expiry: jwt.NewNumericDate(time.Now().Add(time.Hour)), + NotBefore: jwt.NewNumericDate(time.Now().Add(-time.Hour)), + IssuedAt: jwt.NewNumericDate(time.Now().Add(-time.Hour)), + } + var groups interface{} + if test.jwtClaims != nil { + test.jwtClaims(&wellKnownClaims, &groups) + } + + var signingKey interface{} = goodECSigningKey + signingAlgo := goodECSigningAlgo + signingKID := goodECSigningKeyID + if test.jwtSignature != nil { + test.jwtSignature(&signingKey, &signingAlgo, &signingKID) + } + + jwt := createJWT(t, signingKey, signingAlgo, signingKID, &wellKnownClaims, groups) + rsp, authenticated, err := a.AuthenticateToken(context.Background(), jwt) + if test.wantErrorRegexp != "" { + require.Error(t, err) + require.Regexp(t, test.wantErrorRegexp, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, test.wantResponse, rsp) + require.Equal(t, test.wantAuthenticated, authenticated) + } + }) + } +} + +func tlsSpecFromTLSConfig(tls *tls.Config) *auth1alpha1.TLSSpec { + pemData := make([]byte, 0) + for _, certificate := range tls.Certificates { + for _, reallyCertificate := range certificate.Certificate { + pemData = append(pemData, pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: reallyCertificate, + })...) + } + } + return &auth1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString(pemData), + } +} + +func createJWT( + t *testing.T, + signingKey interface{}, + signingAlgo jose.SignatureAlgorithm, + kid string, + claims *jwt.Claims, + groups interface{}, +) string { + t.Helper() + + sig, err := jose.NewSigner( + jose.SigningKey{Algorithm: signingAlgo, Key: signingKey}, + (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", kid), + ) + require.NoError(t, err) + + builder := jwt.Signed(sig).Claims(claims) + if groups != nil { + builder = builder.Claims(map[string]interface{}{"groups": groups}) + } + jwt, err := builder.CompactSerialize() + require.NoError(t, err) + + t.Log("andrew:", jwt) + + return jwt +} diff --git a/internal/controller/authenticator/webhookcachecleaner/webhookcachecleaner.go b/internal/controller/authenticator/webhookcachecleaner/webhookcachecleaner.go deleted file mode 100644 index b8d28acf..00000000 --- a/internal/controller/authenticator/webhookcachecleaner/webhookcachecleaner.go +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Package webhookcachecleaner implements a controller for garbage collecting webhook authenticators from an authenticator cache. -package webhookcachecleaner - -import ( - "fmt" - - "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/klog/v2" - - auth1alpha1 "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1" - authinformers "go.pinniped.dev/generated/1.19/client/concierge/informers/externalversions/authentication/v1alpha1" - pinnipedcontroller "go.pinniped.dev/internal/controller" - "go.pinniped.dev/internal/controller/authenticator/authncache" - "go.pinniped.dev/internal/controllerlib" -) - -// New instantiates a new controllerlib.Controller which will garbage collect webhooks from the provided Cache. -func New(cache *authncache.Cache, webhooks authinformers.WebhookAuthenticatorInformer, log logr.Logger) controllerlib.Controller { - return controllerlib.New( - controllerlib.Config{ - Name: "webhookcachecleaner-controller", - Syncer: &controller{ - cache: cache, - webhooks: webhooks, - log: log.WithName("webhookcachecleaner-controller"), - }, - }, - controllerlib.WithInformer( - webhooks, - pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), - controllerlib.InformerOption{}, - ), - ) -} - -type controller struct { - cache *authncache.Cache - webhooks authinformers.WebhookAuthenticatorInformer - log logr.Logger -} - -// Sync implements controllerlib.Syncer. -func (c *controller) Sync(_ controllerlib.Context) error { - webhooks, err := c.webhooks.Lister().List(labels.Everything()) - if err != nil { - return fmt.Errorf("failed to list WebhookAuthenticators: %w", err) - } - - // Index the current webhooks by key. - webhooksByKey := map[controllerlib.Key]*auth1alpha1.WebhookAuthenticator{} - for _, webhook := range webhooks { - key := controllerlib.Key{Namespace: webhook.Namespace, Name: webhook.Name} - webhooksByKey[key] = webhook - } - - // Delete any entries from the cache which are no longer in the cluster. - for _, key := range c.cache.Keys() { - if key.APIGroup != auth1alpha1.SchemeGroupVersion.Group || key.Kind != "WebhookAuthenticator" { - continue - } - if _, exists := webhooksByKey[controllerlib.Key{Namespace: key.Namespace, Name: key.Name}]; !exists { - c.log.WithValues("webhook", klog.KRef(key.Namespace, key.Name)).Info("deleting webhook authenticator from cache") - c.cache.Delete(key) - } - } - return nil -} diff --git a/internal/controller/authenticator/webhookcachecleaner/webhookcachecleaner_test.go b/internal/controller/authenticator/webhookcachecleaner/webhookcachecleaner_test.go deleted file mode 100644 index 5f1d0674..00000000 --- a/internal/controller/authenticator/webhookcachecleaner/webhookcachecleaner_test.go +++ /dev/null @@ -1,144 +0,0 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package webhookcachecleaner - -import ( - "context" - "testing" - "time" - - "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - - authv1alpha "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1" - pinnipedfake "go.pinniped.dev/generated/1.19/client/concierge/clientset/versioned/fake" - pinnipedinformers "go.pinniped.dev/generated/1.19/client/concierge/informers/externalversions" - "go.pinniped.dev/internal/controller/authenticator/authncache" - "go.pinniped.dev/internal/controllerlib" - "go.pinniped.dev/internal/testutil/testlogger" -) - -func TestController(t *testing.T) { - t.Parallel() - - testKey1 := authncache.Key{ - APIGroup: "authentication.concierge.pinniped.dev", - Kind: "WebhookAuthenticator", - Namespace: "test-namespace", - Name: "test-name-one", - } - testKey2 := authncache.Key{ - APIGroup: "authentication.concierge.pinniped.dev", - Kind: "WebhookAuthenticator", - Namespace: "test-namespace", - Name: "test-name-two", - } - testKeyNonwebhook := authncache.Key{ - APIGroup: "authentication.concierge.pinniped.dev", - Kind: "SomeOtherAuthenticator", - Namespace: "test-namespace", - Name: "test-name-one", - } - - tests := []struct { - name string - webhooks []runtime.Object - initialCache map[authncache.Key]authncache.Value - wantErr string - wantLogs []string - wantCacheKeys []authncache.Key - }{ - { - name: "no change", - initialCache: map[authncache.Key]authncache.Value{testKey1: nil}, - webhooks: []runtime.Object{ - &authv1alpha.WebhookAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testKey1.Namespace, - Name: testKey1.Name, - }, - }, - }, - wantCacheKeys: []authncache.Key{testKey1}, - }, - { - name: "authenticators not yet added", - initialCache: nil, - webhooks: []runtime.Object{ - &authv1alpha.WebhookAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testKey1.Namespace, - Name: testKey1.Name, - }, - }, - &authv1alpha.WebhookAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testKey2.Namespace, - Name: testKey2.Name, - }, - }, - }, - wantCacheKeys: []authncache.Key{}, - }, - { - name: "successful cleanup", - initialCache: map[authncache.Key]authncache.Value{ - testKey1: nil, - testKey2: nil, - testKeyNonwebhook: nil, - }, - webhooks: []runtime.Object{ - &authv1alpha.WebhookAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testKey1.Namespace, - Name: testKey1.Name, - }, - }, - }, - wantLogs: []string{ - `webhookcachecleaner-controller "level"=0 "msg"="deleting webhook authenticator from cache" "webhook"={"name":"test-name-two","namespace":"test-namespace"}`, - }, - wantCacheKeys: []authncache.Key{testKey1, testKeyNonwebhook}, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - fakeClient := pinnipedfake.NewSimpleClientset(tt.webhooks...) - informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0) - cache := authncache.New() - for k, v := range tt.initialCache { - cache.Store(k, v) - } - testLog := testlogger.New(t) - - controller := New(cache, informers.Authentication().V1alpha1().WebhookAuthenticators(), testLog) - - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - - informers.Start(ctx.Done()) - controllerlib.TestRunSynchronously(t, controller) - - syncCtx := controllerlib.Context{ - Context: ctx, - Key: controllerlib.Key{ - Namespace: "test-namespace", - Name: "test-name-one", - }, - } - - if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { - require.EqualError(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - require.Equal(t, tt.wantLogs, testLog.Lines()) - require.ElementsMatch(t, tt.wantCacheKeys, cache.Keys()) - }) - } -} diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index 9dcf0447..34330fcf 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -5,7 +5,6 @@ package webhookcachefiller import ( - "encoding/base64" "fmt" "io/ioutil" "os" @@ -23,6 +22,7 @@ import ( auth1alpha1 "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1" authinformers "go.pinniped.dev/generated/1.19/client/concierge/informers/externalversions/authentication/v1alpha1" pinnipedcontroller "go.pinniped.dev/internal/controller" + pinnipedauthenticator "go.pinniped.dev/internal/controller/authenticator" "go.pinniped.dev/internal/controller/authenticator/authncache" "go.pinniped.dev/internal/controllerlib" ) @@ -92,7 +92,7 @@ func newWebhookAuthenticator( defer func() { _ = os.Remove(temp.Name()) }() cluster := &clientcmdapi.Cluster{Server: spec.Endpoint} - cluster.CertificateAuthorityData, err = getCABundle(spec.TLS) + cluster.CertificateAuthorityData, err = pinnipedauthenticator.CABundle(spec.TLS) if err != nil { return nil, fmt.Errorf("invalid TLS configuration: %w", err) } @@ -121,10 +121,3 @@ func newWebhookAuthenticator( return webhook.New(temp.Name(), version, implicitAuds, customDial) } - -func getCABundle(spec *auth1alpha1.TLSSpec) ([]byte, error) { - if spec == nil { - return nil, nil - } - return base64.StdEncoding.DecodeString(spec.CertificateAuthorityData) -} diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index bb8c3523..22ce266d 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -25,7 +25,8 @@ import ( "go.pinniped.dev/internal/config/concierge" "go.pinniped.dev/internal/controller/apicerts" "go.pinniped.dev/internal/controller/authenticator/authncache" - "go.pinniped.dev/internal/controller/authenticator/webhookcachecleaner" + "go.pinniped.dev/internal/controller/authenticator/cachecleaner" + "go.pinniped.dev/internal/controller/authenticator/jwtcachefiller" "go.pinniped.dev/internal/controller/authenticator/webhookcachefiller" "go.pinniped.dev/internal/controller/issuerconfig" "go.pinniped.dev/internal/controller/kubecertagent" @@ -238,9 +239,18 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { singletonWorker, ). WithController( - webhookcachecleaner.New( + jwtcachefiller.New( + c.AuthenticatorCache, + informers.installationNamespacePinniped.Authentication().V1alpha1().JWTAuthenticators(), + klogr.New(), + ), + singletonWorker, + ). + WithController( + cachecleaner.New( c.AuthenticatorCache, informers.installationNamespacePinniped.Authentication().V1alpha1().WebhookAuthenticators(), + informers.installationNamespacePinniped.Authentication().V1alpha1().JWTAuthenticators(), klogr.New(), ), singletonWorker, diff --git a/internal/registry/credentialrequest/rest.go b/internal/registry/credentialrequest/rest.go index fcbb0516..3e2e6eb3 100644 --- a/internal/registry/credentialrequest/rest.go +++ b/internal/registry/credentialrequest/rest.go @@ -79,7 +79,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation user, err := r.authenticator.AuthenticateTokenCredentialRequest(ctx, credentialRequest) if err != nil { - traceFailureWithError(t, "webhook authentication", err) + traceFailureWithError(t, "token authentication", err) return failureResponse(), nil } if user == nil || user.GetName() == "" { diff --git a/internal/registry/credentialrequest/rest_test.go b/internal/registry/credentialrequest/rest_test.go index 4b311715..032dc681 100644 --- a/internal/registry/credentialrequest/rest_test.go +++ b/internal/registry/credentialrequest/rest_test.go @@ -145,7 +145,7 @@ func TestCreate(t *testing.T) { response, err := callCreate(context.Background(), storage, req) requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) - requireOneLogStatement(r, logger, `"failure" failureType:webhook authentication,msg:some webhook error`) + requireOneLogStatement(r, logger, `"failure" failureType:token authentication,msg:some webhook error`) }) it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookReturnsAnEmptyUsername", func() { diff --git a/test/integration/cli_test.go b/test/integration/cli_test.go index cc51904e..707fe0e7 100644 --- a/test/integration/cli_test.go +++ b/test/integration/cli_test.go @@ -114,17 +114,93 @@ func TestCLILoginOIDC(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) defer cancel() - // Start the browser driver. - page := browsertest.Open(t) - // Build pinniped CLI. t.Logf("building CLI binary") pinnipedExe := buildPinnipedCLI(t) + // Run "pinniped login oidc" to get an ExecCredential struct with an OIDC ID token. + credOutput, sessionCachePath := runPinniedLoginOIDC(ctx, t, pinnipedExe) + + // Assert some properties of the ExecCredential. + t.Logf("validating ExecCredential") + require.NotNil(t, credOutput.Status) + require.Empty(t, credOutput.Status.ClientKeyData) + require.Empty(t, credOutput.Status.ClientCertificateData) + + // There should be at least 1 minute of remaining expiration (probably more). + require.NotNil(t, credOutput.Status.ExpirationTimestamp) + ttl := time.Until(credOutput.Status.ExpirationTimestamp.Time) + require.Greater(t, ttl.Milliseconds(), (1 * time.Minute).Milliseconds()) + + // Assert some properties about the token, which should be a valid JWT. + require.NotEmpty(t, credOutput.Status.Token) + jws, err := jose.ParseSigned(credOutput.Status.Token) + require.NoError(t, err) + claims := map[string]interface{}{} + require.NoError(t, json.Unmarshal(jws.UnsafePayloadWithoutVerification(), &claims)) + require.Equal(t, env.CLITestUpstream.Issuer, claims["iss"]) + require.Equal(t, env.CLITestUpstream.ClientID, claims["aud"]) + require.Equal(t, env.CLITestUpstream.Username, claims["email"]) + require.NotEmpty(t, claims["nonce"]) + + // Run the CLI again with the same session cache and login parameters. + t.Logf("starting second CLI subprocess to test session caching") + cmd2Output, err := oidcLoginCommand(ctx, t, pinnipedExe, sessionCachePath).CombinedOutput() + require.NoError(t, err, string(cmd2Output)) + + // Expect the CLI to output the same ExecCredential in JSON format. + t.Logf("validating second ExecCredential") + var credOutput2 clientauthenticationv1beta1.ExecCredential + require.NoErrorf(t, json.Unmarshal(cmd2Output, &credOutput2), + "command returned something other than an ExecCredential:\n%s", string(cmd2Output)) + require.Equal(t, credOutput, credOutput2) + + // Overwrite the cache entry to remove the access and ID tokens. + t.Logf("overwriting cache to remove valid ID token") + cache := filesession.New(sessionCachePath) + cacheKey := oidcclient.SessionCacheKey{ + Issuer: env.CLITestUpstream.Issuer, + ClientID: env.CLITestUpstream.ClientID, + Scopes: []string{"email", "offline_access", "openid", "profile"}, + RedirectURI: strings.ReplaceAll(env.CLITestUpstream.CallbackURL, "127.0.0.1", "localhost"), + } + cached := cache.GetToken(cacheKey) + require.NotNil(t, cached) + require.NotNil(t, cached.RefreshToken) + require.NotEmpty(t, cached.RefreshToken.Token) + cached.IDToken = nil + cached.AccessToken = nil + cache.PutToken(cacheKey, cached) + + // Run the CLI a third time with the same session cache and login parameters. + t.Logf("starting third CLI subprocess to test refresh flow") + cmd3Output, err := oidcLoginCommand(ctx, t, pinnipedExe, sessionCachePath).CombinedOutput() + require.NoError(t, err, string(cmd2Output)) + + // Expect the CLI to output a new ExecCredential in JSON format (different from the one returned the first two times). + t.Logf("validating third ExecCredential") + var credOutput3 clientauthenticationv1beta1.ExecCredential + require.NoErrorf(t, json.Unmarshal(cmd3Output, &credOutput3), + "command returned something other than an ExecCredential:\n%s", string(cmd2Output)) + require.NotEqual(t, credOutput2.Status.Token, credOutput3.Status.Token) +} + +func runPinniedLoginOIDC( + ctx context.Context, + t *testing.T, + pinnipedExe string, +) (clientauthenticationv1beta1.ExecCredential, string) { + t.Helper() + + env := library.IntegrationEnv(t) + // Make a temp directory to hold the session cache for this test. sessionCachePath := testutil.TempDir(t) + "/sessions.yaml" - // Start the CLI running the "alpha login oidc [...]" command with stdout/stderr connected to pipes. + // Start the browser driver. + page := browsertest.Open(t) + + // Start the CLI running the "login oidc [...]" command with stdout/stderr connected to pipes. cmd := oidcLoginCommand(ctx, t, pinnipedExe, sessionCachePath) stderr, err := cmd.StderrPipe() require.NoError(t, err) @@ -221,68 +297,7 @@ func TestCLILoginOIDC(t *testing.T) { case credOutput = <-credOutputChan: } - // Assert some properties of the ExecCredential. - t.Logf("validating ExecCredential") - require.NotNil(t, credOutput.Status) - require.Empty(t, credOutput.Status.ClientKeyData) - require.Empty(t, credOutput.Status.ClientCertificateData) - - // There should be at least 1 minute of remaining expiration (probably more). - require.NotNil(t, credOutput.Status.ExpirationTimestamp) - ttl := time.Until(credOutput.Status.ExpirationTimestamp.Time) - require.Greater(t, ttl.Milliseconds(), (1 * time.Minute).Milliseconds()) - - // Assert some properties about the token, which should be a valid JWT. - require.NotEmpty(t, credOutput.Status.Token) - jws, err := jose.ParseSigned(credOutput.Status.Token) - require.NoError(t, err) - claims := map[string]interface{}{} - require.NoError(t, json.Unmarshal(jws.UnsafePayloadWithoutVerification(), &claims)) - require.Equal(t, env.CLITestUpstream.Issuer, claims["iss"]) - require.Equal(t, env.CLITestUpstream.ClientID, claims["aud"]) - require.Equal(t, env.CLITestUpstream.Username, claims["email"]) - require.NotEmpty(t, claims["nonce"]) - - // Run the CLI again with the same session cache and login parameters. - t.Logf("starting second CLI subprocess to test session caching") - cmd2Output, err := oidcLoginCommand(ctx, t, pinnipedExe, sessionCachePath).CombinedOutput() - require.NoError(t, err, string(cmd2Output)) - - // Expect the CLI to output the same ExecCredential in JSON format. - t.Logf("validating second ExecCredential") - var credOutput2 clientauthenticationv1beta1.ExecCredential - require.NoErrorf(t, json.Unmarshal(cmd2Output, &credOutput2), - "command returned something other than an ExecCredential:\n%s", string(cmd2Output)) - require.Equal(t, credOutput, credOutput2) - - // Overwrite the cache entry to remove the access and ID tokens. - t.Logf("overwriting cache to remove valid ID token") - cache := filesession.New(sessionCachePath) - cacheKey := oidcclient.SessionCacheKey{ - Issuer: env.CLITestUpstream.Issuer, - ClientID: env.CLITestUpstream.ClientID, - Scopes: []string{"email", "offline_access", "openid", "profile"}, - RedirectURI: strings.ReplaceAll(env.CLITestUpstream.CallbackURL, "127.0.0.1", "localhost"), - } - cached := cache.GetToken(cacheKey) - require.NotNil(t, cached) - require.NotNil(t, cached.RefreshToken) - require.NotEmpty(t, cached.RefreshToken.Token) - cached.IDToken = nil - cached.AccessToken = nil - cache.PutToken(cacheKey, cached) - - // Run the CLI a third time with the same session cache and login parameters. - t.Logf("starting third CLI subprocess to test refresh flow") - cmd3Output, err := oidcLoginCommand(ctx, t, pinnipedExe, sessionCachePath).CombinedOutput() - require.NoError(t, err, string(cmd2Output)) - - // Expect the CLI to output a new ExecCredential in JSON format (different from the one returned the first two times). - t.Logf("validating third ExecCredential") - var credOutput3 clientauthenticationv1beta1.ExecCredential - require.NoErrorf(t, json.Unmarshal(cmd3Output, &credOutput3), - "command returned something other than an ExecCredential:\n%s", string(cmd2Output)) - require.NotEqual(t, credOutput2.Status.Token, credOutput3.Status.Token) + return credOutput, sessionCachePath } func readAndExpectEmpty(r io.Reader) (err error) { diff --git a/test/integration/concierge_credentialrequest_test.go b/test/integration/concierge_credentialrequest_test.go index 30ca7eb3..5e2458e0 100644 --- a/test/integration/concierge_credentialrequest_test.go +++ b/test/integration/concierge_credentialrequest_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + jwtpkg "gopkg.in/square/go-jose.v2/jwt" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,48 +45,89 @@ func TestSuccessfulCredentialRequest(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 6*time.Minute) defer cancel() - testWebhook := library.CreateTestWebhookAuthenticator(ctx, t) + tests := []struct { + name string + authenticator func(t *testing.T) corev1.TypedLocalObjectReference + token func(t *testing.T) (token string, username string, groups []string) + }{ + { + name: "webhook", + authenticator: func(t *testing.T) corev1.TypedLocalObjectReference { + return library.CreateTestWebhookAuthenticator(ctx, t) + }, + token: func(t *testing.T) (string, string, []string) { + return library.IntegrationEnv(t).TestUser.Token, env.TestUser.ExpectedUsername, env.TestUser.ExpectedGroups + }, + }, + { + name: "jwt authenticator", + authenticator: func(t *testing.T) corev1.TypedLocalObjectReference { + return library.CreateTestJWTAuthenticator(ctx, t) + }, + token: func(t *testing.T) (string, string, []string) { + pinnipedExe := buildPinnipedCLI(t) + credOutput, _ := runPinniedLoginOIDC(ctx, t, pinnipedExe) + token := credOutput.Status.Token - var response *loginv1alpha1.TokenCredentialRequest - successfulResponse := func() bool { - var err error - response, err = makeRequest(ctx, t, validCredentialRequestSpecWithRealToken(t, testWebhook)) - require.NoError(t, err, "the request should never fail at the HTTP level") - return response.Status.Credential != nil + // By default, the JWTAuthenticator expects the username to be in the "sub" claim and the + // groups to be in the "groups" claim. + username, groups := getJWTSubAndGroupsClaims(t, token) + + return credOutput.Status.Token, username, groups + }, + }, } - assert.Eventually(t, successfulResponse, 10*time.Second, 500*time.Millisecond) - require.NotNil(t, response) - require.NotNil(t, response.Status.Credential) - require.Empty(t, response.Status.Message) - require.Empty(t, response.Spec) - require.Empty(t, response.Status.Credential.Token) - require.NotEmpty(t, response.Status.Credential.ClientCertificateData) - require.Equal(t, env.TestUser.ExpectedUsername, getCommonName(t, response.Status.Credential.ClientCertificateData)) - require.ElementsMatch(t, env.TestUser.ExpectedGroups, getOrganizations(t, response.Status.Credential.ClientCertificateData)) - require.NotEmpty(t, response.Status.Credential.ClientKeyData) - require.NotNil(t, response.Status.Credential.ExpirationTimestamp) - require.InDelta(t, 5*time.Minute, time.Until(response.Status.Credential.ExpirationTimestamp.Time), float64(time.Minute)) + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + authenticator := test.authenticator(t) + token, username, groups := test.token(t) - // Create a client using the admin kubeconfig. - adminClient := library.NewClientset(t) + var response *loginv1alpha1.TokenCredentialRequest + successfulResponse := func() bool { + var err error + response, err = makeRequest(ctx, t, loginv1alpha1.TokenCredentialRequestSpec{ + Token: token, + Authenticator: authenticator, + }) + require.NoError(t, err, "the request should never fail at the HTTP level") + return response.Status.Credential != nil + } + assert.Eventually(t, successfulResponse, 10*time.Second, 500*time.Millisecond) + require.NotNil(t, response) + require.Emptyf(t, response.Status.Message, "value is: %q", safeDerefStringPtr(response.Status.Message)) + require.NotNil(t, response.Status.Credential) + require.Empty(t, response.Spec) + require.Empty(t, response.Status.Credential.Token) + require.NotEmpty(t, response.Status.Credential.ClientCertificateData) + require.Equal(t, username, getCommonName(t, response.Status.Credential.ClientCertificateData)) + require.ElementsMatch(t, groups, getOrganizations(t, response.Status.Credential.ClientCertificateData)) + require.NotEmpty(t, response.Status.Credential.ClientKeyData) + require.NotNil(t, response.Status.Credential.ExpirationTimestamp) + require.InDelta(t, 5*time.Minute, time.Until(response.Status.Credential.ExpirationTimestamp.Time), float64(time.Minute)) - // Create a client using the certificate from the CredentialRequest. - clientWithCertFromCredentialRequest := library.NewClientsetWithCertAndKey( - t, - response.Status.Credential.ClientCertificateData, - response.Status.Credential.ClientKeyData, - ) + // Create a client using the admin kubeconfig. + adminClient := library.NewClientset(t) - t.Run( - "access as user", - library.AccessAsUserTest(ctx, adminClient, env.TestUser.ExpectedUsername, clientWithCertFromCredentialRequest), - ) - for _, group := range env.TestUser.ExpectedGroups { - group := group - t.Run( - "access as group "+group, - library.AccessAsGroupTest(ctx, adminClient, group, clientWithCertFromCredentialRequest), - ) + // Create a client using the certificate from the CredentialRequest. + clientWithCertFromCredentialRequest := library.NewClientsetWithCertAndKey( + t, + response.Status.Credential.ClientCertificateData, + response.Status.Credential.ClientKeyData, + ) + + t.Run( + "access as user", + library.AccessAsUserTest(ctx, adminClient, username, clientWithCertFromCredentialRequest), + ) + for _, group := range groups { + group := group + t.Run( + "access as group "+group, + library.AccessAsGroupTest(ctx, adminClient, group, clientWithCertFromCredentialRequest), + ) + } + }) } } @@ -183,3 +225,26 @@ func getOrganizations(t *testing.T, certPEM string) []string { return cert.Subject.Organization } + +func safeDerefStringPtr(s *string) string { + if s == nil { + return "" + } + return *s +} + +func getJWTSubAndGroupsClaims(t *testing.T, jwt string) (string, []string) { + t.Helper() + + token, err := jwtpkg.ParseSigned(jwt) + require.NoError(t, err) + + var claims struct { + Sub string `json:"sub"` + Groups []string `json:"groups"` + } + err = token.UnsafeClaimsWithoutVerification(&claims) + require.NoError(t, err) + + return claims.Sub, claims.Groups +} diff --git a/test/library/client.go b/test/library/client.go index b151b86b..7a24b54d 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -6,6 +6,7 @@ package library import ( "context" "crypto/rand" + "encoding/base64" "encoding/hex" "fmt" "io" @@ -163,6 +164,51 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty } } +// CreateTestJWTAuthenticator creates and returns a test JWTAuthenticator in +// $PINNIPED_TEST_CONCIERGE_NAMESPACE, which will be automatically deleted at the end of the current +// test's lifetime. It returns a corev1.TypedLocalObjectReference which describes the test JWT +// authenticator within the test namespace. +// +// CreateTestJWTAuthenticator gets the OIDC issuer info from IntegrationEnv().CLITestUpstream. +func CreateTestJWTAuthenticator(ctx context.Context, t *testing.T) corev1.TypedLocalObjectReference { + t.Helper() + testEnv := IntegrationEnv(t) + + client := NewConciergeClientset(t) + jwtAuthenticators := client.AuthenticationV1alpha1().JWTAuthenticators(testEnv.ConciergeNamespace) + + createContext, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + jwtAuthenticator, err := jwtAuthenticators.Create(createContext, &auth1alpha1.JWTAuthenticator{ + ObjectMeta: testObjectMeta(t, "jwt-authenticator"), + Spec: auth1alpha1.JWTAuthenticatorSpec{ + Issuer: testEnv.CLITestUpstream.Issuer, + Audience: testEnv.CLITestUpstream.ClientID, + TLS: &auth1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(testEnv.CLITestUpstream.CABundle)), + }, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err, "could not create test JWTAuthenticator") + t.Logf("created test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name) + + t.Cleanup(func() { + t.Helper() + t.Logf("cleaning up test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name) + deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err := jwtAuthenticators.Delete(deleteCtx, jwtAuthenticator.Name, metav1.DeleteOptions{}) + require.NoErrorf(t, err, "could not cleanup test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name) + }) + + return corev1.TypedLocalObjectReference{ + APIGroup: &auth1alpha1.SchemeGroupVersion.Group, + Kind: "JWTAuthenticator", + Name: jwtAuthenticator.Name, + } +} + // CreateTestOIDCProvider creates and returns a test OIDCProvider in // $PINNIPED_TEST_SUPERVISOR_NAMESPACE, which will be automatically deleted at the end of the // current test's lifetime. It generates a random, valid, issuer for the OIDCProvider.