diff --git a/go.sum b/go.sum index 21f57d39..a776e664 100644 --- a/go.sum +++ b/go.sum @@ -873,6 +873,8 @@ k8s.io/apiserver v0.19.2 h1:xq2dXAzsAoHv7S4Xc/p7PKhiowdHV/PgdePWo3MxIYM= k8s.io/apiserver v0.19.2/go.mod h1:FreAq0bJ2vtZFj9Ago/X0oNGC51GfubKK/ViOKfVAOA= k8s.io/client-go v0.19.2 h1:gMJuU3xJZs86L1oQ99R4EViAADUPMHHtS9jFshasHSc= k8s.io/client-go v0.19.2/go.mod h1:S5wPhCqyDNAlzM9CnEdgTGV4OqhsW3jGO1UM1epwfJA= +k8s.io/client-go v1.5.1 h1:XaX/lo2/u3/pmFau8HN+sB5C/b4dc4Dmm2eXjBH4p1E= +k8s.io/client-go v11.0.0+incompatible h1:LBbX2+lOwY9flffWlJM7f1Ct8V2SRNiMRDFeiwnJo9o= k8s.io/code-generator v0.19.2/go.mod h1:moqLn7w0t9cMs4+5CQyxnfA/HV8MF6aAVENF+WZZhgk= k8s.io/component-base v0.19.2 h1:jW5Y9RcZTb79liEhW3XDVTW7MuvEGP0tQZnfSX6/+gs= k8s.io/component-base v0.19.2/go.mod h1:g5LrsiTiabMLZ40AR6Hl45f088DevyGY+cCE2agEIVo= diff --git a/hack/lib/tilt/Tiltfile b/hack/lib/tilt/Tiltfile index 26213238..6af66bcd 100644 --- a/hack/lib/tilt/Tiltfile +++ b/hack/lib/tilt/Tiltfile @@ -71,7 +71,7 @@ k8s_yaml(local([ '--data-value', 'namespace=supervisor', '--data-value', 'image_repo=image/supervisor', '--data-value', 'image_tag=tilt-dev', - '--data-value-yaml', 'replicas=1', + '--data-value-yaml', 'replicas=2', '--data-value-yaml', 'service_nodeport_port=31234', ])) diff --git a/internal/controller/supervisorconfig/jwks.go b/internal/controller/supervisorconfig/jwks.go index f2b730ba..0ba44b72 100644 --- a/internal/controller/supervisorconfig/jwks.go +++ b/internal/controller/supervisorconfig/jwks.go @@ -9,6 +9,7 @@ import ( "crypto/rsa" "encoding/json" "fmt" + "io" "gopkg.in/square/go-jose.v2" corev1 "k8s.io/api/core/v1" @@ -33,16 +34,24 @@ const ( // // Note! The value for this key will contain private key material! activeJWKKey = "activeJWK" - // validJWKSKey points to the current JWKS used to verify tokens. + // jwksKey points to the current JWKS used to verify tokens. // // Note! The value for this key will contain only public key material! - validJWKSKey = "validJWKS" + jwksKey = "jwks" ) const ( opcKind = "OIDCProviderConfig" ) +// generateKey is stubbed out for the purpose of testing. The default behavior is to generate an RSA key. +//nolint:gochecknoglobals +var generateKey func(r io.Reader, bits int) (interface{}, error) = generateRSAKey + +func generateRSAKey(r io.Reader, bits int) (interface{}, error) { + return rsa.GenerateKey(r, bits) +} + // jwkController holds the field necessary for the JWKS controller to communicate with OPC's and // secrets, both via a cache and via the API. type jwksController struct { @@ -86,15 +95,11 @@ func NewJWKSController( } return controllerlib.Key{} }, - AddFunc: func(obj metav1.Object) bool { - return false - }, + AddFunc: isOPCControllee, UpdateFunc: func(oldObj, newObj metav1.Object) bool { return isOPCControllee(oldObj) || isOPCControllee(newObj) }, - DeleteFunc: func(obj metav1.Object) bool { - return isOPCControllee(obj) - }, + DeleteFunc: isOPCControllee, }, controllerlib.InformerOption{}, ), @@ -131,28 +136,26 @@ func (c *jwksController) Sync(ctx controllerlib.Context) error { return nil } - var secret *corev1.Secret - if opc.Status.JWKSSecret.Name != "" { - // This OPC says it has a secret associated with it. Let's try to get it from the cache. - secret, err = c.secretInformer.Lister().Secrets(opc.Namespace).Get(opc.Status.JWKSSecret.Name) - if err != nil && !k8serrors.IsNotFound(err) { - return fmt.Errorf("cannot get secret: %w", err) - } + secretNeedsUpdate, err := c.secretNeedsUpdate(opc) + if err != nil { + return fmt.Errorf("cannot determine secret status: %w", err) + } + if !secretNeedsUpdate { + // Secret is up to date - we are good to go. + return nil } - if secret == nil { - // If the OPC does not have a secret associated with it, or that secret does not exist, we will - // generate a new secret (i.e., a JWKS). - secret, err = c.generateSecret(opc) - if err != nil { - return fmt.Errorf("cannot wire secret: %w", err) - } + // If the OPC does not have a secret associated with it, that secret does not exist, or the secret + // is invalid, we will generate a new secret (i.e., a JWKS). + secret, err := c.generateSecret(opc) + if err != nil { + return fmt.Errorf("cannot generate secret: %w", err) } - // Ensure that the secret exists and looks like it should. if err := c.createOrUpdateSecret(ctx.Context, secret); err != nil { return fmt.Errorf("cannot create or update secret: %w", err) } + klog.InfoS("created/updated secret", "secret", klog.KObj(secret)) // Ensure that the OPC points to the secret. @@ -166,6 +169,31 @@ func (c *jwksController) Sync(ctx controllerlib.Context) error { return nil } +func (c *jwksController) secretNeedsUpdate(opc *configv1alpha1.OIDCProviderConfig) (bool, error) { + if opc.Status.JWKSSecret.Name == "" { + // If the OPC says it doesn't have a secret associated with it, then let's create one. + return true, nil + } + + // This OPC says it has a secret associated with it. Let's try to get it from the cache. + secret, err := c.secretInformer.Lister().Secrets(opc.Namespace).Get(opc.Status.JWKSSecret.Name) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return false, fmt.Errorf("cannot get secret: %w", err) + } + if notFound { + // If we can't find the secret, let's assume we need to create it. + return true, nil + } + + if !isValid(secret) { + // If this secret is invalid, we need to generate a new one. + return true, nil + } + + return false, nil +} + func (c *jwksController) generateSecret(opc *configv1alpha1.OIDCProviderConfig) (*corev1.Secret, error) { // Note! This is where we could potentially add more handling of OPC spec fields which tell us how // this OIDC provider should sign and verify ID tokens (e.g., hardcoded token secret, gRPC @@ -173,13 +201,13 @@ func (c *jwksController) generateSecret(opc *configv1alpha1.OIDCProviderConfig) // // For now, we just generate an new RSA keypair and put that in the secret. - privateKey, err := rsa.GenerateKey(rand.Reader, 4096) + key, err := generateKey(rand.Reader, 4096) if err != nil { return nil, fmt.Errorf("cannot generate key: %w", err) } jwk := jose.JSONWebKey{ - Key: privateKey, + Key: key, KeyID: "some-key", Algorithm: "RS256", Use: "sig", @@ -190,7 +218,7 @@ func (c *jwksController) generateSecret(opc *configv1alpha1.OIDCProviderConfig) } jwks := jose.JSONWebKeySet{ - Keys: []jose.JSONWebKey{jwk}, + Keys: []jose.JSONWebKey{jwk.Public()}, } jwksData, err := json.Marshal(jwks) if err != nil { @@ -212,7 +240,7 @@ func (c *jwksController) generateSecret(opc *configv1alpha1.OIDCProviderConfig) }, Data: map[string][]byte{ activeJWKKey: jwkData, - validJWKSKey: jwksData, + jwksKey: jwksData, }, } @@ -260,9 +288,8 @@ func (c *jwksController) updateOPC( opcClient := c.pinnipedClient.ConfigV1alpha1().OIDCProviderConfigs(newOPC.Namespace) return retry.RetryOnConflict(retry.DefaultRetry, func() error { oldOPC, err := opcClient.Get(ctx, newOPC.Name, metav1.GetOptions{}) - notFound := k8serrors.IsNotFound(err) - if err != nil && !notFound { - return fmt.Errorf("cannot get secret: %w", err) + if err != nil { + return fmt.Errorf("cannot get opc: %w", err) } if newOPC.Status.JWKSSecret.Name == oldOPC.Status.JWKSSecret.Name { @@ -279,7 +306,9 @@ func (c *jwksController) updateOPC( // isOPCControlle returns whether the provided obj is controlled by an OPC. func isOPCControllee(obj metav1.Object) bool { controller := metav1.GetControllerOf(obj) - return controller != nil && controller.Kind == opcKind + return controller != nil && + controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() && + controller.Kind == opcKind } // isValid returns whether the provided secret contains a valid active JWK and verification JWKS. @@ -296,12 +325,17 @@ func isValid(secret *corev1.Secret) bool { return false } + if activeJWK.IsPublic() { + klog.InfoS("active jwk is public", "keyid", activeJWK.KeyID) + return false + } + if !activeJWK.Valid() { klog.InfoS("active jwk is not valid", "keyid", activeJWK.KeyID) return false } - jwksData, ok := secret.Data[validJWKSKey] + jwksData, ok := secret.Data[jwksKey] if !ok { klog.InfoS("secret does not contain valid jwks") } @@ -314,6 +348,10 @@ func isValid(secret *corev1.Secret) bool { foundActiveJWK := false for _, validJWK := range validJWKS.Keys { + if !validJWK.IsPublic() { + klog.InfoS("jwks key is not public", "keyid", validJWK.KeyID) + return false + } if !validJWK.Valid() { klog.InfoS("jwks key is not valid", "keyid", validJWK.KeyID) return false diff --git a/internal/controller/supervisorconfig/jwks_test.go b/internal/controller/supervisorconfig/jwks_test.go index 7770a4e2..30298db4 100644 --- a/internal/controller/supervisorconfig/jwks_test.go +++ b/internal/controller/supervisorconfig/jwks_test.go @@ -3,7 +3,697 @@ package supervisorconfig -import "testing" +import ( + "bytes" + "context" + "crypto/x509" + "encoding/pem" + "errors" + "io" + "io/ioutil" + "testing" + "time" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + kubeinformers "k8s.io/client-go/informers" + kubernetesfake "k8s.io/client-go/kubernetes/fake" + kubetesting "k8s.io/client-go/testing" + + configv1alpha1 "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" + pinnipedfake "go.pinniped.dev/generated/1.19/client/clientset/versioned/fake" + pinnipedinformers "go.pinniped.dev/generated/1.19/client/informers/externalversions" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/testutil" +) + +func TestJWKSControllerFilterSecret(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + secret corev1.Secret + wantAdd bool + wantUpdate bool + wantDelete bool + wantParent controllerlib.Key + }{ + { + name: "no owner reference", + secret: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{}, + }, + }, + { + name: "owner reference without correct APIVersion", + secret: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "some-namespace", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "OIDCProviderConfig", + Name: "some-name", + Controller: boolPtr(true), + }, + }, + }, + }, + }, + { + name: "owner reference without correct Kind", + secret: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "some-namespace", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: configv1alpha1.SchemeGroupVersion.String(), + Name: "some-name", + Controller: boolPtr(true), + }, + }, + }, + }, + }, + { + name: "owner reference without controller set to true", + secret: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "some-namespace", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: configv1alpha1.SchemeGroupVersion.String(), + Kind: "OIDCProviderConfig", + Name: "some-name", + }, + }, + }, + }, + }, + { + name: "correct owner reference", + secret: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "some-namespace", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: configv1alpha1.SchemeGroupVersion.String(), + Kind: "OIDCProviderConfig", + Name: "some-name", + Controller: boolPtr(true), + }, + }, + }, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + wantParent: controllerlib.Key{Namespace: "some-namespace", Name: "some-name"}, + }, + { + name: "multiple owner references", + secret: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "some-namespace", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "UnrelatedKind", + }, + { + APIVersion: configv1alpha1.SchemeGroupVersion.String(), + Kind: "OIDCProviderConfig", + Name: "some-name", + Controller: boolPtr(true), + }, + }, + }, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + wantParent: controllerlib.Key{Namespace: "some-namespace", Name: "some-name"}, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + secretInformer := kubeinformers.NewSharedInformerFactory( + kubernetesfake.NewSimpleClientset(), + 0, + ).Core().V1().Secrets() + opcInformer := pinnipedinformers.NewSharedInformerFactory( + pinnipedfake.NewSimpleClientset(), + 0, + ).Config().V1alpha1().OIDCProviderConfigs() + withInformer := testutil.NewObservableWithInformerOption() + _ = NewJWKSController( + nil, // kubeClient, not needed + nil, // pinnipedClient, not needed + secretInformer, + opcInformer, + withInformer.WithInformer, + ) + + unrelated := corev1.Secret{} + filter := withInformer.GetFilterForInformer(secretInformer) + require.Equal(t, test.wantAdd, filter.Add(&test.secret)) + require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.secret)) + require.Equal(t, test.wantUpdate, filter.Update(&test.secret, &unrelated)) + require.Equal(t, test.wantDelete, filter.Delete(&test.secret)) + require.Equal(t, test.wantParent, filter.Parent(&test.secret)) + }) + } +} + +func TestJWKSControllerFilterOPC(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + opc configv1alpha1.OIDCProviderConfig + wantAdd bool + wantUpdate bool + wantDelete bool + wantParent controllerlib.Key + }{ + { + name: "anything goes", + opc: configv1alpha1.OIDCProviderConfig{}, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + wantParent: controllerlib.Key{}, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + secretInformer := kubeinformers.NewSharedInformerFactory( + kubernetesfake.NewSimpleClientset(), + 0, + ).Core().V1().Secrets() + opcInformer := pinnipedinformers.NewSharedInformerFactory( + pinnipedfake.NewSimpleClientset(), + 0, + ).Config().V1alpha1().OIDCProviderConfigs() + withInformer := testutil.NewObservableWithInformerOption() + _ = NewJWKSController( + nil, // kubeClient, not needed + nil, // pinnipedClient, not needed + secretInformer, + opcInformer, + withInformer.WithInformer, + ) + + unrelated := configv1alpha1.OIDCProviderConfig{} + filter := withInformer.GetFilterForInformer(opcInformer) + require.Equal(t, test.wantAdd, filter.Add(&test.opc)) + require.Equal(t, test.wantUpdate, filter.Update(&unrelated, &test.opc)) + require.Equal(t, test.wantUpdate, filter.Update(&test.opc, &unrelated)) + require.Equal(t, test.wantDelete, filter.Delete(&test.opc)) + require.Equal(t, test.wantParent, filter.Parent(&test.opc)) + }) + } +} func TestJWKSControllerSync(t *testing.T) { + // We shouldn't run this test in parallel since it messes with a global function (generateKey). + + const namespace = "tuna-namespace" + + goodRSAKeyPEM, err := ioutil.ReadFile("testdata/good-rsa-key.pem") + require.NoError(t, err) + block, _ := pem.Decode(goodRSAKeyPEM) + require.NotNil(t, block, "expected block to be non-nil...is goodRSAKeyPEM a valid PEM?") + goodRSAKey, err := x509.ParsePKCS1PrivateKey(block.Bytes) + require.NoError(t, err) + + opcGVR := schema.GroupVersionResource{ + Group: configv1alpha1.SchemeGroupVersion.Group, + Version: configv1alpha1.SchemeGroupVersion.Version, + Resource: "oidcproviderconfigs", + } + + goodOPC := &configv1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "good-opc", + Namespace: namespace, + UID: "good-opc-uid", + }, + Spec: configv1alpha1.OIDCProviderConfigSpec{ + Issuer: "https://some-issuer.com", + }, + } + goodOPCWithStatus := goodOPC.DeepCopy() + goodOPCWithStatus.Status.JWKSSecret.Name = goodOPCWithStatus.Name + "-jwks" + + secretGVR := schema.GroupVersionResource{ + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + Resource: "secrets", + } + + newSecret := func(activeJWKPath, jwksPath string) *corev1.Secret { + s := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodOPCWithStatus.Status.JWKSSecret.Name, + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: opcGVR.GroupVersion().String(), + Kind: "OIDCProviderConfig", + Name: goodOPC.Name, + UID: goodOPC.UID, + BlockOwnerDeletion: boolPtr(true), + Controller: boolPtr(true), + }, + }, + }, + } + s.Data = make(map[string][]byte) + if activeJWKPath != "" { + s.Data["activeJWK"] = read(t, activeJWKPath) + } + if jwksPath != "" { + s.Data["jwks"] = read(t, jwksPath) + } + return &s + } + + goodSecret := newSecret("testdata/good-jwk.json", "testdata/good-jwks.json") + + tests := []struct { + name string + key controllerlib.Key + secrets []*corev1.Secret + configKubeClient func(*kubernetesfake.Clientset) + configPinnipedClient func(*pinnipedfake.Clientset) + opcs []*configv1alpha1.OIDCProviderConfig + generateKeyErr error + wantGenerateKeyCount int + wantSecretActions []kubetesting.Action + wantOPCActions []kubetesting.Action + wantError string + }{ + { + name: "new opc with no secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPC, + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + kubetesting.NewUpdateAction(opcGVR, namespace, goodOPCWithStatus), + }, + }, + { + name: "opc without status with existing secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPC, + }, + secrets: []*corev1.Secret{ + goodSecret, + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + kubetesting.NewUpdateAction(opcGVR, namespace, goodOPCWithStatus), + }, + }, + { + name: "existing opc with no secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "existing opc with existing secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + goodSecret, + }, + }, + { + name: "deleted opc", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + // Nothing to do here since Kube will garbage collect our child secret via its OwnerReference. + }, + { + name: "missing jwk in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("", "testdata/good-jwks.json"), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "missing jwks in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("testdata/good-jwk.json", ""), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "invalid jwk JSON in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("testdata/not-json.txt", "testdata/good-jwks.json"), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "invalid jwks JSON in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("testdata/good-jwk.json", "testdata/not-json.txt"), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "public jwk in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("testdata/public-jwk.json", "testdata/good-jwks.json"), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "private jwks in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("testdata/good-jwk.json", "testdata/private-jwks.json"), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "invalid jwk key in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("testdata/invalid-key-jwk.json", "testdata/good-jwks.json"), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "invalid jwks key in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("testdata/good-jwk.json", "testdata/invalid-key-jwks.json"), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "missing active jwks in secret", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + secrets: []*corev1.Secret{ + newSecret("testdata/good-jwk.json", "testdata/missing-active-jwks.json"), + }, + wantGenerateKeyCount: 1, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantOPCActions: []kubetesting.Action{ + kubetesting.NewGetAction(opcGVR, namespace, goodOPC.Name), + }, + }, + { + name: "generate key fails", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPCWithStatus, + }, + generateKeyErr: errors.New("some generate error"), + wantError: "cannot generate secret: cannot generate key: some generate error", + }, + { + name: "get secret fails", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPC, + }, + configKubeClient: func(client *kubernetesfake.Clientset) { + client.PrependReactor("get", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some get error") + }) + }, + wantError: "cannot create or update secret: cannot get secret: some get error", + }, + { + name: "create secret fails", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPC, + }, + configKubeClient: func(client *kubernetesfake.Clientset) { + client.PrependReactor("create", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some create error") + }) + }, + wantError: "cannot create or update secret: cannot create secret: some create error", + }, + { + name: "update secret fails", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPC, + }, + secrets: []*corev1.Secret{ + newSecret("", ""), + }, + configKubeClient: func(client *kubernetesfake.Clientset) { + client.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some update error") + }) + }, + wantError: "cannot create or update secret: some update error", + }, + { + name: "get opc fails", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPC, + }, + configPinnipedClient: func(client *pinnipedfake.Clientset) { + client.PrependReactor("get", "oidcproviderconfigs", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some get error") + }) + }, + wantError: "cannot update opc: cannot get opc: some get error", + }, + { + name: "update opc fails", + key: controllerlib.Key{Namespace: goodOPC.Namespace, Name: goodOPC.Name}, + opcs: []*configv1alpha1.OIDCProviderConfig{ + goodOPC, + }, + configPinnipedClient: func(client *pinnipedfake.Clientset) { + client.PrependReactor("update", "oidcproviderconfigs", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some update error") + }) + }, + wantError: "cannot update opc: some update error", + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + // We shouldn't run this test in parallel since it messes with a global function (generateKey). + generateKeyCount := 0 + generateKey = func(_ io.Reader, _ int) (interface{}, error) { + generateKeyCount++ + return goodRSAKey, test.generateKeyErr + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) + defer cancel() + + kubeAPIClient := kubernetesfake.NewSimpleClientset() + kubeInformerClient := kubernetesfake.NewSimpleClientset() + for _, secret := range test.secrets { + require.NoError(t, kubeAPIClient.Tracker().Add(secret)) + require.NoError(t, kubeInformerClient.Tracker().Add(secret)) + } + if test.configKubeClient != nil { + test.configKubeClient(kubeAPIClient) + } + + pinnipedAPIClient := pinnipedfake.NewSimpleClientset() + pinnipedInformerClient := pinnipedfake.NewSimpleClientset() + for _, opc := range test.opcs { + require.NoError(t, pinnipedAPIClient.Tracker().Add(opc)) + require.NoError(t, pinnipedInformerClient.Tracker().Add(opc)) + } + if test.configPinnipedClient != nil { + test.configPinnipedClient(pinnipedAPIClient) + } + + kubeInformers := kubeinformers.NewSharedInformerFactory( + kubeInformerClient, + 0, + ) + pinnipedInformers := pinnipedinformers.NewSharedInformerFactory( + pinnipedInformerClient, + 0, + ) + + c := NewJWKSController( + kubeAPIClient, + pinnipedAPIClient, + kubeInformers.Core().V1().Secrets(), + pinnipedInformers.Config().V1alpha1().OIDCProviderConfigs(), + controllerlib.WithInformer, + ) + + // Must start informers before calling TestRunSynchronously(). + kubeInformers.Start(ctx.Done()) + pinnipedInformers.Start(ctx.Done()) + controllerlib.TestRunSynchronously(t, c) + + err := controllerlib.TestSync(t, c, controllerlib.Context{ + Context: ctx, + Key: test.key, + }) + if test.wantError != "" { + require.EqualError(t, err, test.wantError) + return + } + require.NoError(t, err) + + require.Equal(t, test.wantGenerateKeyCount, generateKeyCount) + + if test.wantSecretActions != nil { + require.Equal(t, test.wantSecretActions, kubeAPIClient.Actions()) + } + if test.wantOPCActions != nil { + require.Equal(t, test.wantOPCActions, pinnipedAPIClient.Actions()) + } + }) + } } + +func read(t *testing.T, path string) []byte { + t.Helper() + + data, err := ioutil.ReadFile(path) + require.NoError(t, err) + + // Trim whitespace from our testdata so that we match the compact JSON encoding of + // our implementation. + data = bytes.ReplaceAll(data, []byte(" "), []byte{}) + data = bytes.ReplaceAll(data, []byte("\n"), []byte{}) + + return data +} + +func boolPtr(b bool) *bool { return &b } diff --git a/internal/controller/supervisorconfig/testdata/good-jwk.json b/internal/controller/supervisorconfig/testdata/good-jwk.json new file mode 100644 index 00000000..20cda4f3 --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/good-jwk.json @@ -0,0 +1,14 @@ +{ + "use": "sig", + "kty": "RSA", + "kid": "some-key", + "alg": "RS256", + "n": "z6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo2XshthG21-L82rmxUQ23-1XTkBSBK5iZl3Q_liHt1MLjZrpjuRc0CKMDcrExAMX6duicFVlhkIakIeupp-PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa-c2GXuEuX572CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnAdxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B_Q0g3vGyFNVf0MM4LX3OSre4yVlphZOW3YeLIeBq_4KmgutD0AZHzCF18KUjJgOv9w", + "e": "AQAB", + "d": "yIaLQBD3CzgkRcsdeZN7LLTmL8BHcw-kPEul3WLtPmUBvJsiEfUBd0zgINjKi4gsnzP6azRVXZ0PqURzf3n6NkiJ36Bd70UtLQAldfnHSKmpwy9uVAsLQOrSd7ovI7rsWoCXcW0K1p70lSEcbJYLRlJEipDuLM1aC1iHNAyGEcuQr4vlKaaWJ0lwQv0dxeEYsOTvMUvewOy1T8gREdSOQYJ5PgcF6solq04gCYmGv2paEersPFcfEarA5h8FHKlqGRTGwg3ltJMA1NaRPs0teYR2nKdLUk8nc012F7qfpN8iDx6H6f8tJn_QchgbLo8_s5uB6KC2zmdceCLRiP-VQQ", + "p": "53FGQ4Kc-bJeZfRejxgg1avgi0i7THXpb2_-E2hgUpzFEza2e3TOQ1-N44sotDVjv7bylZwuLsdV7ug9jIVWzr4qldEOlpvGOh_QjqrEl12lwA-9EFNp3UrMwclGUvLwm5QjFRE74iEQR0b0ljetvupNE-FncNWhxlOnQEDXVjE", + "q": "5a1kYSkozQiPHEycYpuflRWRY_twrWywhO2Gwzqq583qBUYInUjhNS6_dzrAA_6rDSuPXux4OjxdkLbVziLfHhLo_f1fCTm0-UlQIasxfn-WTFRpZsAjzaaiL5n0OegvSDAKXbT9zuAfH5r6RjhsSXqG-s5jbk810rVmwUh2Vqc", + "dp": "bhRnaga-qNjYoz-GliLQwzA73aObSjOu8szemNaFMeXUql3Uj4Wv8UWKlBaFJqlaJz5ZxSUCpkczLS2S0Lo-3ph-YsGLYcD3mH-3T5QTazckdeRGdXRnHtTL7MPRyfQ40paz1PpcdCJrvqsV_DjBT9PbE0CbVYSWrGDvZNUyVpE", + "dq": "4jjKASVQSbtfcklHU5zjLy3COc-EaVz_9L4cGZlkktNv6GfVvk31fLOh5OcaEBU8F8nK-n1B4mJo6kwcBWC1kOKhWOLCQ8zyIwQCCFeddXJn8KDH_GvOGBZD80zZkFvQjnK7ExddUvHP1gqI7rdOeYVVBB5bM2CTrAn-vuwHm0s", + "qi": "brSwOeUadJ9wnqNN_cdCKyDb8ed37h7Cd509hkiby7JiD7VqBfFWmYqtIdX-jEfms6OSlCiUKAeTHryKAV7Wb6yHNgT78iOCfgGIIz2mmV8KNdAzdkkMlGu5Uuwi0EW8ww25Xw0c5zIneVZmg-0ydFUUa5GEHrQ3Du7MMAHlQCo" +} diff --git a/internal/controller/supervisorconfig/testdata/good-jwks.json b/internal/controller/supervisorconfig/testdata/good-jwks.json new file mode 100644 index 00000000..ddb4fecb --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/good-jwks.json @@ -0,0 +1,12 @@ +{ + "keys": [ + { + "use": "sig", + "kty": "RSA", + "kid": "some-key", + "alg": "RS256", + "n": "z6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo2XshthG21-L82rmxUQ23-1XTkBSBK5iZl3Q_liHt1MLjZrpjuRc0CKMDcrExAMX6duicFVlhkIakIeupp-PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa-c2GXuEuX572CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnAdxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B_Q0g3vGyFNVf0MM4LX3OSre4yVlphZOW3YeLIeBq_4KmgutD0AZHzCF18KUjJgOv9w", + "e": "AQAB" + } + ] +} diff --git a/internal/controller/supervisorconfig/testdata/good-rsa-key.pem b/internal/controller/supervisorconfig/testdata/good-rsa-key.pem new file mode 100644 index 00000000..459761eb --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/good-rsa-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEAz6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo +2XshthG21+L82rmxUQ23+1XTkBSBK5iZl3Q/liHt1MLjZrpjuRc0CKMDcrExAMX6 +duicFVlhkIakIeupp+PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa+c2GXuEuX5 +72CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnA +dxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B/Q0g3vGyFNVf0MM4LX3OSre4yVlp +hZOW3YeLIeBq/4KmgutD0AZHzCF18KUjJgOv9wIDAQABAoIBAQDIhotAEPcLOCRF +yx15k3sstOYvwEdzD6Q8S6XdYu0+ZQG8myIR9QF3TOAg2MqLiCyfM/prNFVdnQ+p +RHN/efo2SInfoF3vRS0tACV1+cdIqanDL25UCwtA6tJ3ui8juuxagJdxbQrWnvSV +IRxslgtGUkSKkO4szVoLWIc0DIYRy5Cvi+UpppYnSXBC/R3F4Riw5O8xS97A7LVP +yBER1I5Bgnk+BwXqyiWrTiAJiYa/aloR6uw8Vx8RqsDmHwUcqWoZFMbCDeW0kwDU +1pE+zS15hHacp0tSTydzTXYXup+k3yIPHofp/y0mf9ByGBsujz+zm4HooLbOZ1x4 +ItGI/5VBAoGBAOdxRkOCnPmyXmX0Xo8YINWr4ItIu0x16W9v/hNoYFKcxRM2tnt0 +zkNfjeOLKLQ1Y7+28pWcLi7HVe7oPYyFVs6+KpXRDpabxjof0I6qxJddpcAPvRBT +ad1KzMHJRlLy8JuUIxURO+IhEEdG9JY3rb7qTRPhZ3DVocZTp0BA11YxAoGBAOWt +ZGEpKM0IjxxMnGKbn5UVkWP7cK1ssITthsM6qufN6gVGCJ1I4TUuv3c6wAP+qw0r +j17seDo8XZC21c4i3x4S6P39Xwk5tPlJUCGrMX5/lkxUaWbAI82moi+Z9DnoL0gw +Cl20/c7gHx+a+kY4bEl6hvrOY25PNdK1ZsFIdlanAoGAbhRnaga+qNjYoz+GliLQ +wzA73aObSjOu8szemNaFMeXUql3Uj4Wv8UWKlBaFJqlaJz5ZxSUCpkczLS2S0Lo+ +3ph+YsGLYcD3mH+3T5QTazckdeRGdXRnHtTL7MPRyfQ40paz1PpcdCJrvqsV/DjB +T9PbE0CbVYSWrGDvZNUyVpECgYEA4jjKASVQSbtfcklHU5zjLy3COc+EaVz/9L4c +GZlkktNv6GfVvk31fLOh5OcaEBU8F8nK+n1B4mJo6kwcBWC1kOKhWOLCQ8zyIwQC +CFeddXJn8KDH/GvOGBZD80zZkFvQjnK7ExddUvHP1gqI7rdOeYVVBB5bM2CTrAn+ +vuwHm0sCgYButLA55Rp0n3Ceo039x0IrINvx53fuHsJ3nT2GSJvLsmIPtWoF8VaZ +iq0h1f6MR+azo5KUKJQoB5MevIoBXtZvrIc2BPvyI4J+AYgjPaaZXwo10DN2SQyU +a7lS7CLQRbzDDblfDRznMid5VmaD7TJ0VRRrkYQetDcO7swwAeVAKg== +-----END RSA PRIVATE KEY----- diff --git a/internal/controller/supervisorconfig/testdata/invalid-key-jwk.json b/internal/controller/supervisorconfig/testdata/invalid-key-jwk.json new file mode 100644 index 00000000..4c51e5c9 --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/invalid-key-jwk.json @@ -0,0 +1,8 @@ +{ + "use": "sig", + "kty": "RSA", + "kid": "some-key", + "alg": "RS256", + "n": "z6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo2XshthG21-L82rmxUQ23-1XTkBSBK5iZl3Q_liHt1MLjZrpjuRc0CKMDcrExAMX6duicFVlhkIakIeupp-PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa-c2GXuEuX572CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnAdxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B_Q0g3vGyFNVf0MM4LX3OSre4yVlphZOW3YeLIeBq_4KmgutD0AZHzCF18KUjJgOv9w", + "e": "0" +} diff --git a/internal/controller/supervisorconfig/testdata/invalid-key-jwks.json b/internal/controller/supervisorconfig/testdata/invalid-key-jwks.json new file mode 100644 index 00000000..b5eef40e --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/invalid-key-jwks.json @@ -0,0 +1,12 @@ +{ + "keys": [ + { + "use": "sig", + "kty": "RSA", + "kid": "some-key", + "alg": "RS256", + "n": "0", + "e": "AQAB" + } + ] +} diff --git a/internal/controller/supervisorconfig/testdata/missing-active-jwks.json b/internal/controller/supervisorconfig/testdata/missing-active-jwks.json new file mode 100644 index 00000000..08e36e4f --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/missing-active-jwks.json @@ -0,0 +1,12 @@ +{ + "keys": [ + { + "use": "sig", + "kty": "RSA", + "kid": "some-other-key", + "alg": "RS256", + "n": "qNAsShEVuXiPz2UmI-1q_R_80WA3VHWt7WU7NbhPf59GohTKKvosG4a1C8alY2eh25yFIB6BbyPOFnTWFDrPnNmZYn0m0ByHW7EbO92yFKjS6F9p1VICWOp003F5UWIfCy5fzFA3oDBPSBs2r6N9g0xcqbwihuT1Cn1vQb_CRA0-G44XFQ4hHnHJfmFsgv-za7BlcT4V_RRaPtJBNnQRVmNXxjKwLs1XwGAW-I0QObr4HPsMBdBPXJYQeC5WJS59KbP2wvimgkArzStdw-n2H_5TYUaKFyylX8vCb3ndCs7Mp90fI3YGhvZrQ7N7mmL_vn4lrCcQMD2T_U9-dKbB6aXXNlyS-VY-MXbhnY_MGbGIGEdIdwGynGmyuLiNCA9qXDJ4zVWdlatsTqSFyGh20ntj8fcdxfjMg_AXbwr_Fc_9dkvshU9Qsui6FCxB6GwZA4o9Pyu0NtzetWcwZdpKpDaFTkmhQbPMP6MoshovaYdJWYsvuBSjTZycawikgMWAPuinFSAcwI10P6YucJRVlUgIOMusKnGfu8xXxQWysleesJe-1BSQHmyKjIGuIIjiWamAga8Hn4n24LqlBhRgjPJqL_QH25GrpIyFW-6DsHuOKNgJk7IJSZOl6Mkox660gsbdfpTsYeEY9IWc5am4vZOfadx86d9O13p7rZBUsus", + "e": "AQAB" + } + ] +} diff --git a/internal/controller/supervisorconfig/testdata/not-json.txt b/internal/controller/supervisorconfig/testdata/not-json.txt new file mode 100644 index 00000000..78f5ab28 --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/not-json.txt @@ -0,0 +1 @@ +this ain't json diff --git a/internal/controller/supervisorconfig/testdata/private-jwks.json b/internal/controller/supervisorconfig/testdata/private-jwks.json new file mode 100644 index 00000000..d72ce22b --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/private-jwks.json @@ -0,0 +1,18 @@ +{ + "keys": [ + { + "use": "sig", + "kty": "RSA", + "kid": "some-key", + "alg": "RS256", + "n": "z6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo2XshthG21-L82rmxUQ23-1XTkBSBK5iZl3Q_liHt1MLjZrpjuRc0CKMDcrExAMX6duicFVlhkIakIeupp-PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa-c2GXuEuX572CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnAdxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B_Q0g3vGyFNVf0MM4LX3OSre4yVlphZOW3YeLIeBq_4KmgutD0AZHzCF18KUjJgOv9w", + "e": "AQAB", + "d": "yIaLQBD3CzgkRcsdeZN7LLTmL8BHcw-kPEul3WLtPmUBvJsiEfUBd0zgINjKi4gsnzP6azRVXZ0PqURzf3n6NkiJ36Bd70UtLQAldfnHSKmpwy9uVAsLQOrSd7ovI7rsWoCXcW0K1p70lSEcbJYLRlJEipDuLM1aC1iHNAyGEcuQr4vlKaaWJ0lwQv0dxeEYsOTvMUvewOy1T8gREdSOQYJ5PgcF6solq04gCYmGv2paEersPFcfEarA5h8FHKlqGRTGwg3ltJMA1NaRPs0teYR2nKdLUk8nc012F7qfpN8iDx6H6f8tJn_QchgbLo8_s5uB6KC2zmdceCLRiP-VQQ", + "p": "53FGQ4Kc-bJeZfRejxgg1avgi0i7THXpb2_-E2hgUpzFEza2e3TOQ1-N44sotDVjv7bylZwuLsdV7ug9jIVWzr4qldEOlpvGOh_QjqrEl12lwA-9EFNp3UrMwclGUvLwm5QjFRE74iEQR0b0ljetvupNE-FncNWhxlOnQEDXVjE", + "q": "5a1kYSkozQiPHEycYpuflRWRY_twrWywhO2Gwzqq583qBUYInUjhNS6_dzrAA_6rDSuPXux4OjxdkLbVziLfHhLo_f1fCTm0-UlQIasxfn-WTFRpZsAjzaaiL5n0OegvSDAKXbT9zuAfH5r6RjhsSXqG-s5jbk810rVmwUh2Vqc", + "dp": "bhRnaga-qNjYoz-GliLQwzA73aObSjOu8szemNaFMeXUql3Uj4Wv8UWKlBaFJqlaJz5ZxSUCpkczLS2S0Lo-3ph-YsGLYcD3mH-3T5QTazckdeRGdXRnHtTL7MPRyfQ40paz1PpcdCJrvqsV_DjBT9PbE0CbVYSWrGDvZNUyVpE", + "dq": "4jjKASVQSbtfcklHU5zjLy3COc-EaVz_9L4cGZlkktNv6GfVvk31fLOh5OcaEBU8F8nK-n1B4mJo6kwcBWC1kOKhWOLCQ8zyIwQCCFeddXJn8KDH_GvOGBZD80zZkFvQjnK7ExddUvHP1gqI7rdOeYVVBB5bM2CTrAn-vuwHm0s", + "qi": "brSwOeUadJ9wnqNN_cdCKyDb8ed37h7Cd509hkiby7JiD7VqBfFWmYqtIdX-jEfms6OSlCiUKAeTHryKAV7Wb6yHNgT78iOCfgGIIz2mmV8KNdAzdkkMlGu5Uuwi0EW8ww25Xw0c5zIneVZmg-0ydFUUa5GEHrQ3Du7MMAHlQCo" + } + ] +} diff --git a/internal/controller/supervisorconfig/testdata/public-jwk.json b/internal/controller/supervisorconfig/testdata/public-jwk.json new file mode 100644 index 00000000..1b4b7a88 --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/public-jwk.json @@ -0,0 +1,8 @@ +{ + "use": "sig", + "kty": "RSA", + "kid": "some-key", + "alg": "RS256", + "n": "z6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo2XshthG21-L82rmxUQ23-1XTkBSBK5iZl3Q_liHt1MLjZrpjuRc0CKMDcrExAMX6duicFVlhkIakIeupp-PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa-c2GXuEuX572CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnAdxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B_Q0g3vGyFNVf0MM4LX3OSre4yVlphZOW3YeLIeBq_4KmgutD0AZHzCF18KUjJgOv9w", + "e": "AQAB" +} diff --git a/test/integration/supervisor_keys_test.go b/test/integration/supervisor_keys_test.go new file mode 100644 index 00000000..09c720ec --- /dev/null +++ b/test/integration/supervisor_keys_test.go @@ -0,0 +1,96 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package integration + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/square/go-jose.v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + configv1alpha1 "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" + "go.pinniped.dev/test/library" +) + +func TestSupervisorOIDCKeys(t *testing.T) { + env := library.IntegrationEnv(t) + kubeClient := library.NewClientset(t) + pinnipedClient := library.NewPinnipedClientset(t) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + + // Create our OPC under test. + // TODO: maybe use this in other supervisor test? + opc := library.CreateTestOIDCProvider(ctx, t) + + // Ensure a secret is created with the OPC's JWKS. + var updatedOPC *configv1alpha1.OIDCProviderConfig + var err error + assert.Eventually(t, func() bool { + updatedOPC, err = pinnipedClient. + ConfigV1alpha1(). + OIDCProviderConfigs(env.SupervisorNamespace). + Get(ctx, opc.Name, metav1.GetOptions{}) + return err == nil && updatedOPC.Status.JWKSSecret.Name != "" + }, time.Second*10, time.Millisecond*500) + require.NoError(t, err) + require.NotEmpty(t, updatedOPC.Status.JWKSSecret.Name) + + // Ensure the secret actually exists. + secret, err := kubeClient. + CoreV1(). + Secrets(env.SupervisorNamespace). + Get(ctx, updatedOPC.Status.JWKSSecret.Name, metav1.GetOptions{}) + require.NoError(t, err) + + // Ensure the secret has an active key. + jwkData, ok := secret.Data["activeJWK"] + require.True(t, ok, "secret is missing active jwk") + + // Ensure the secret's active key is valid. + var activeJWK jose.JSONWebKey + require.NoError(t, json.Unmarshal(jwkData, &activeJWK)) + require.True(t, activeJWK.Valid(), "active jwk is invalid") + require.False(t, activeJWK.IsPublic(), "active jwk is public") + + // Ensure the secret has a JWKS. + jwksData, ok := secret.Data["jwks"] + require.True(t, ok, "secret is missing jwks") + + // Ensure the secret's JWKS is valid, public, and contains the singing key. + var jwks jose.JSONWebKeySet + require.NoError(t, json.Unmarshal(jwksData, &jwks)) + foundActiveJWK := false + for _, jwk := range jwks.Keys { + require.Truef(t, jwk.Valid(), "jwk %s is invalid", jwk.KeyID) + require.Truef(t, jwk.IsPublic(), "jwk %s is not public", jwk.KeyID) + if jwk.KeyID == activeJWK.KeyID { + foundActiveJWK = true + } + } + require.True(t, foundActiveJWK, "could not find active JWK in JWKS: %s", jwks) + + // Ensure upon deleting the secret, it is eventually brought back. + err = kubeClient. + CoreV1(). + Secrets(env.SupervisorNamespace). + Delete(ctx, updatedOPC.Status.JWKSSecret.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + assert.Eventually(t, func() bool { + secret, err = kubeClient. + CoreV1(). + Secrets(env.SupervisorNamespace). + Get(ctx, updatedOPC.Status.JWKSSecret.Name, metav1.GetOptions{}) + return err == nil + }, time.Second*10, time.Millisecond*500) + require.NoError(t, err) + + // Upon deleting the OPC, the secret is deleted (we test this behavior in our uninstall tests). +} diff --git a/test/library/client.go b/test/library/client.go index a5cdcdb3..6ecc0ce8 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -5,11 +5,16 @@ package library import ( "context" + "crypto/rand" + "encoding/hex" + "fmt" + "io" "io/ioutil" "os" "testing" "time" + "github.com/pkg/errors" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -18,6 +23,7 @@ import ( "k8s.io/client-go/tools/clientcmd" aggregatorclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" + configv1alpha1 "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned" @@ -152,3 +158,50 @@ func CreateTestWebhookIDP(ctx context.Context, t *testing.T) corev1.TypedLocalOb Name: idp.Name, } } + +// CreateTestOIDCProvider creates and returns a test OIDCProviderConfig 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 OIDCProviderConfig. +func CreateTestOIDCProvider(ctx context.Context, t *testing.T) *configv1alpha1.OIDCProviderConfig { + t.Helper() + testEnv := IntegrationEnv(t) + + createContext, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + issuer, err := randomIssuer() + require.NoError(t, err) + + opcs := NewPinnipedClientset(t).ConfigV1alpha1().OIDCProviderConfigs(testEnv.SupervisorNamespace) + opc, err := opcs.Create(createContext, &configv1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-oidc-provider-", + Labels: map[string]string{"pinniped.dev/test": ""}, + Annotations: map[string]string{"pinniped.dev/testName": t.Name()}, + }, + Spec: configv1alpha1.OIDCProviderConfigSpec{ + Issuer: issuer, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err, "could not create test OIDCProviderConfig") + t.Logf("created test OIDCProviderConfig %s/%s", opc.Namespace, opc.Name) + + t.Cleanup(func() { + t.Helper() + t.Logf("cleaning up test OIDCProviderConfig %s/%s", opc.Namespace, opc.Name) + deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err := opcs.Delete(deleteCtx, opc.Name, metav1.DeleteOptions{}) + require.NoErrorf(t, err, "could not cleanup test OIDCProviderConfig %s/%s", opc.Namespace, opc.Name) + }) + + return opc +} + +func randomIssuer() (string, error) { + var buf [8]byte + if _, err := io.ReadFull(rand.Reader, buf[:]); err != nil { + return "", errors.WithMessage(err, "could not generate random state") + } + return fmt.Sprintf("http://test-issuer-%s.pinniped.dev", hex.EncodeToString(buf[:])), nil +}