From 6aed025c796f211bd8d857bf5938b419afec4e24 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Wed, 14 Oct 2020 09:47:34 -0400 Subject: [PATCH 01/10] supervisor-generate-key: initial spike Signed-off-by: Andrew Keesler --- .../v1alpha1/types_oidcproviderconfig.go.tmpl | 21 +- cmd/pinniped-supervisor/main.go | 41 ++- ...nfig.pinniped.dev_oidcproviderconfigs.yaml | 11 + deploy/supervisor/rbac.yaml | 3 + generated/1.17/README.adoc | 1 + .../v1alpha1/types_oidcproviderconfig.go | 21 +- .../config/v1alpha1/zz_generated.deepcopy.go | 1 + .../client/openapi/zz_generated.openapi.go | 8 +- ...nfig.pinniped.dev_oidcproviderconfigs.yaml | 11 + generated/1.18/README.adoc | 1 + .../v1alpha1/types_oidcproviderconfig.go | 21 +- .../config/v1alpha1/zz_generated.deepcopy.go | 1 + .../client/openapi/zz_generated.openapi.go | 8 +- ...nfig.pinniped.dev_oidcproviderconfigs.yaml | 11 + generated/1.19/README.adoc | 1 + .../v1alpha1/types_oidcproviderconfig.go | 21 +- .../config/v1alpha1/zz_generated.deepcopy.go | 1 + .../client/openapi/zz_generated.openapi.go | 8 +- ...nfig.pinniped.dev_oidcproviderconfigs.yaml | 11 + go.mod | 1 + go.sum | 2 + internal/controller/supervisorconfig/jwks.go | 332 ++++++++++++++++++ .../controller/supervisorconfig/jwks_test.go | 9 + 23 files changed, 512 insertions(+), 34 deletions(-) create mode 100644 internal/controller/supervisorconfig/jwks.go create mode 100644 internal/controller/supervisorconfig/jwks_test.go diff --git a/apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl b/apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl index a05f4339..b8b34560 100644 --- a/apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl +++ b/apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl @@ -3,7 +3,10 @@ package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +kubebuilder:validation:Enum=Success;Duplicate;Invalid type OIDCProviderStatus string @@ -39,11 +42,17 @@ type OIDCProviderConfigStatus struct { // +optional Message string `json:"message,omitempty"` - // LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get - // around some undesirable behavior with respect to the empty metav1.Time value (see - // https://github.com/kubernetes/kubernetes/issues/86811). - // +optional - LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + // LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get + // around some undesirable behavior with respect to the empty metav1.Time value (see + // https://github.com/kubernetes/kubernetes/issues/86811). + // +optional + LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + + // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys + // are stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. + // +optional + JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` } // OIDCProviderConfig describes the configuration of an OIDC provider. diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index fe04cd7f..445c9461 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -13,6 +13,8 @@ import ( "time" "k8s.io/apimachinery/pkg/util/clock" + kubeinformers "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/pkg/version" "k8s.io/client-go/rest" restclient "k8s.io/client-go/rest" @@ -62,7 +64,9 @@ func waitForSignal() os.Signal { func startControllers( ctx context.Context, issuerProvider *manager.Manager, + kubeClient kubernetes.Interface, pinnipedClient pinnipedclientset.Interface, + kubeInformers kubeinformers.SharedInformerFactory, pinnipedInformers pinnipedinformers.SharedInformerFactory, ) { // Create controller manager. @@ -77,37 +81,60 @@ func startControllers( controllerlib.WithInformer, ), singletonWorker, + ). + WithController( + supervisorconfig.NewJWKSController( + kubeClient, + pinnipedClient, + kubeInformers.Core().V1().Secrets(), + pinnipedInformers.Config().V1alpha1().OIDCProviderConfigs(), + controllerlib.WithInformer, + ), + singletonWorker, ) + kubeInformers.Start(ctx.Done()) pinnipedInformers.Start(ctx.Done()) go controllerManager.Start(ctx) } -func newPinnipedClient() (pinnipedclientset.Interface, error) { +func newClients() (kubernetes.Interface, pinnipedclientset.Interface, error) { kubeConfig, err := restclient.InClusterConfig() if err != nil { - return nil, fmt.Errorf("could not load in-cluster configuration: %w", err) + return nil, nil, fmt.Errorf("could not load in-cluster configuration: %w", err) } // Connect to the core Kubernetes API. - pinnipedClient, err := pinnipedclientset.NewForConfig(kubeConfig) + kubeClient, err := kubernetes.NewForConfig(kubeConfig) if err != nil { - return nil, fmt.Errorf("could not load in-cluster configuration: %w", err) + return nil, nil, fmt.Errorf("could not create kube client: %w", err) } - return pinnipedClient, nil + // Connect to the Pinniped API. + pinnipedClient, err := pinnipedclientset.NewForConfig(kubeConfig) + if err != nil { + return nil, nil, fmt.Errorf("could not create pinniped client: %w", err) + } + + return kubeClient, pinnipedClient, nil } func run(serverInstallationNamespace string) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - pinnipedClient, err := newPinnipedClient() + kubeClient, pinnipedClient, err := newClients() if err != nil { return fmt.Errorf("cannot create k8s client: %w", err) } + kubeInformers := kubeinformers.NewSharedInformerFactoryWithOptions( + kubeClient, + defaultResyncInterval, + kubeinformers.WithNamespace(serverInstallationNamespace), + ) + pinnipedInformers := pinnipedinformers.NewSharedInformerFactoryWithOptions( pinnipedClient, defaultResyncInterval, @@ -115,7 +142,7 @@ func run(serverInstallationNamespace string) error { ) oidProvidersManager := manager.NewManager(http.NotFoundHandler()) - startControllers(ctx, oidProvidersManager, pinnipedClient, pinnipedInformers) + startControllers(ctx, oidProvidersManager, kubeClient, pinnipedClient, kubeInformers, pinnipedInformers) //nolint: gosec // Intentionally binding to all network interfaces. l, err := net.Listen("tcp", ":80") diff --git a/deploy/supervisor/config.pinniped.dev_oidcproviderconfigs.yaml b/deploy/supervisor/config.pinniped.dev_oidcproviderconfigs.yaml index 062dca9f..727f20ec 100644 --- a/deploy/supervisor/config.pinniped.dev_oidcproviderconfigs.yaml +++ b/deploy/supervisor/config.pinniped.dev_oidcproviderconfigs.yaml @@ -55,6 +55,17 @@ spec: status: description: Status of the OIDC provider. properties: + jwksSecret: + description: JWKSSecret holds the name of the secret in which this + OIDC Provider's signing/verification keys are stored. If it is empty, + then the signing/verification keys are either unknown or they don't + exist. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object lastUpdateTime: description: LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior diff --git a/deploy/supervisor/rbac.yaml b/deploy/supervisor/rbac.yaml index bfa4ac95..4066e139 100644 --- a/deploy/supervisor/rbac.yaml +++ b/deploy/supervisor/rbac.yaml @@ -13,6 +13,9 @@ metadata: labels: app: #@ data.values.app_name rules: + - apiGroups: [""] + resources: [secrets] + verbs: [create, get, list, patch, update, watch, delete] - apiGroups: [config.pinniped.dev] resources: [oidcproviderconfigs] verbs: [update, get, list, watch] diff --git a/generated/1.17/README.adoc b/generated/1.17/README.adoc index c74ee743..bc3adc11 100644 --- a/generated/1.17/README.adoc +++ b/generated/1.17/README.adoc @@ -151,6 +151,7 @@ OIDCProviderConfigStatus is a struct that describes the actual state of an OIDC | *`status`* __OIDCProviderStatus__ | Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. | *`message`* __string__ | Message provides human-readable details about the Status. | *`lastUpdateTime`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#time-v1-meta[$$Time$$]__ | LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior with respect to the empty metav1.Time value (see https://github.com/kubernetes/kubernetes/issues/86811). +| *`jwksSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. |=== diff --git a/generated/1.17/apis/config/v1alpha1/types_oidcproviderconfig.go b/generated/1.17/apis/config/v1alpha1/types_oidcproviderconfig.go index a05f4339..b8b34560 100644 --- a/generated/1.17/apis/config/v1alpha1/types_oidcproviderconfig.go +++ b/generated/1.17/apis/config/v1alpha1/types_oidcproviderconfig.go @@ -3,7 +3,10 @@ package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +kubebuilder:validation:Enum=Success;Duplicate;Invalid type OIDCProviderStatus string @@ -39,11 +42,17 @@ type OIDCProviderConfigStatus struct { // +optional Message string `json:"message,omitempty"` - // LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get - // around some undesirable behavior with respect to the empty metav1.Time value (see - // https://github.com/kubernetes/kubernetes/issues/86811). - // +optional - LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + // LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get + // around some undesirable behavior with respect to the empty metav1.Time value (see + // https://github.com/kubernetes/kubernetes/issues/86811). + // +optional + LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + + // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys + // are stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. + // +optional + JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` } // OIDCProviderConfig describes the configuration of an OIDC provider. diff --git a/generated/1.17/apis/config/v1alpha1/zz_generated.deepcopy.go b/generated/1.17/apis/config/v1alpha1/zz_generated.deepcopy.go index 262992cb..e2522c24 100644 --- a/generated/1.17/apis/config/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.17/apis/config/v1alpha1/zz_generated.deepcopy.go @@ -216,6 +216,7 @@ func (in *OIDCProviderConfigStatus) DeepCopyInto(out *OIDCProviderConfigStatus) in, out := &in.LastUpdateTime, &out.LastUpdateTime *out = (*in).DeepCopy() } + out.JWKSSecret = in.JWKSSecret return } diff --git a/generated/1.17/client/openapi/zz_generated.openapi.go b/generated/1.17/client/openapi/zz_generated.openapi.go index 8e5ad5b8..5d451690 100644 --- a/generated/1.17/client/openapi/zz_generated.openapi.go +++ b/generated/1.17/client/openapi/zz_generated.openapi.go @@ -431,11 +431,17 @@ func schema_117_apis_config_v1alpha1_OIDCProviderConfigStatus(ref common.Referen Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), }, }, + "jwksSecret": { + SchemaProps: spec.SchemaProps{ + Description: "JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist.", + Ref: ref("k8s.io/api/core/v1.LocalObjectReference"), + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, + "k8s.io/api/core/v1.LocalObjectReference", "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, } } diff --git a/generated/1.17/crds/config.pinniped.dev_oidcproviderconfigs.yaml b/generated/1.17/crds/config.pinniped.dev_oidcproviderconfigs.yaml index 062dca9f..727f20ec 100644 --- a/generated/1.17/crds/config.pinniped.dev_oidcproviderconfigs.yaml +++ b/generated/1.17/crds/config.pinniped.dev_oidcproviderconfigs.yaml @@ -55,6 +55,17 @@ spec: status: description: Status of the OIDC provider. properties: + jwksSecret: + description: JWKSSecret holds the name of the secret in which this + OIDC Provider's signing/verification keys are stored. If it is empty, + then the signing/verification keys are either unknown or they don't + exist. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object lastUpdateTime: description: LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior diff --git a/generated/1.18/README.adoc b/generated/1.18/README.adoc index eaa1e5bf..d0b69f73 100644 --- a/generated/1.18/README.adoc +++ b/generated/1.18/README.adoc @@ -151,6 +151,7 @@ OIDCProviderConfigStatus is a struct that describes the actual state of an OIDC | *`status`* __OIDCProviderStatus__ | Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. | *`message`* __string__ | Message provides human-readable details about the Status. | *`lastUpdateTime`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#time-v1-meta[$$Time$$]__ | LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior with respect to the empty metav1.Time value (see https://github.com/kubernetes/kubernetes/issues/86811). +| *`jwksSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. |=== diff --git a/generated/1.18/apis/config/v1alpha1/types_oidcproviderconfig.go b/generated/1.18/apis/config/v1alpha1/types_oidcproviderconfig.go index a05f4339..b8b34560 100644 --- a/generated/1.18/apis/config/v1alpha1/types_oidcproviderconfig.go +++ b/generated/1.18/apis/config/v1alpha1/types_oidcproviderconfig.go @@ -3,7 +3,10 @@ package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +kubebuilder:validation:Enum=Success;Duplicate;Invalid type OIDCProviderStatus string @@ -39,11 +42,17 @@ type OIDCProviderConfigStatus struct { // +optional Message string `json:"message,omitempty"` - // LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get - // around some undesirable behavior with respect to the empty metav1.Time value (see - // https://github.com/kubernetes/kubernetes/issues/86811). - // +optional - LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + // LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get + // around some undesirable behavior with respect to the empty metav1.Time value (see + // https://github.com/kubernetes/kubernetes/issues/86811). + // +optional + LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + + // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys + // are stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. + // +optional + JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` } // OIDCProviderConfig describes the configuration of an OIDC provider. diff --git a/generated/1.18/apis/config/v1alpha1/zz_generated.deepcopy.go b/generated/1.18/apis/config/v1alpha1/zz_generated.deepcopy.go index 262992cb..e2522c24 100644 --- a/generated/1.18/apis/config/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.18/apis/config/v1alpha1/zz_generated.deepcopy.go @@ -216,6 +216,7 @@ func (in *OIDCProviderConfigStatus) DeepCopyInto(out *OIDCProviderConfigStatus) in, out := &in.LastUpdateTime, &out.LastUpdateTime *out = (*in).DeepCopy() } + out.JWKSSecret = in.JWKSSecret return } diff --git a/generated/1.18/client/openapi/zz_generated.openapi.go b/generated/1.18/client/openapi/zz_generated.openapi.go index 75393f9a..bd28cdfd 100644 --- a/generated/1.18/client/openapi/zz_generated.openapi.go +++ b/generated/1.18/client/openapi/zz_generated.openapi.go @@ -431,11 +431,17 @@ func schema_118_apis_config_v1alpha1_OIDCProviderConfigStatus(ref common.Referen Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), }, }, + "jwksSecret": { + SchemaProps: spec.SchemaProps{ + Description: "JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist.", + Ref: ref("k8s.io/api/core/v1.LocalObjectReference"), + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, + "k8s.io/api/core/v1.LocalObjectReference", "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, } } diff --git a/generated/1.18/crds/config.pinniped.dev_oidcproviderconfigs.yaml b/generated/1.18/crds/config.pinniped.dev_oidcproviderconfigs.yaml index 062dca9f..727f20ec 100644 --- a/generated/1.18/crds/config.pinniped.dev_oidcproviderconfigs.yaml +++ b/generated/1.18/crds/config.pinniped.dev_oidcproviderconfigs.yaml @@ -55,6 +55,17 @@ spec: status: description: Status of the OIDC provider. properties: + jwksSecret: + description: JWKSSecret holds the name of the secret in which this + OIDC Provider's signing/verification keys are stored. If it is empty, + then the signing/verification keys are either unknown or they don't + exist. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object lastUpdateTime: description: LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior diff --git a/generated/1.19/README.adoc b/generated/1.19/README.adoc index 1dc10f4c..2e5b95fa 100644 --- a/generated/1.19/README.adoc +++ b/generated/1.19/README.adoc @@ -151,6 +151,7 @@ OIDCProviderConfigStatus is a struct that describes the actual state of an OIDC | *`status`* __OIDCProviderStatus__ | Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. | *`message`* __string__ | Message provides human-readable details about the Status. | *`lastUpdateTime`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#time-v1-meta[$$Time$$]__ | LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior with respect to the empty metav1.Time value (see https://github.com/kubernetes/kubernetes/issues/86811). +| *`jwksSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. |=== diff --git a/generated/1.19/apis/config/v1alpha1/types_oidcproviderconfig.go b/generated/1.19/apis/config/v1alpha1/types_oidcproviderconfig.go index a05f4339..b8b34560 100644 --- a/generated/1.19/apis/config/v1alpha1/types_oidcproviderconfig.go +++ b/generated/1.19/apis/config/v1alpha1/types_oidcproviderconfig.go @@ -3,7 +3,10 @@ package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +kubebuilder:validation:Enum=Success;Duplicate;Invalid type OIDCProviderStatus string @@ -39,11 +42,17 @@ type OIDCProviderConfigStatus struct { // +optional Message string `json:"message,omitempty"` - // LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get - // around some undesirable behavior with respect to the empty metav1.Time value (see - // https://github.com/kubernetes/kubernetes/issues/86811). - // +optional - LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + // LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get + // around some undesirable behavior with respect to the empty metav1.Time value (see + // https://github.com/kubernetes/kubernetes/issues/86811). + // +optional + LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + + // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys + // are stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. + // +optional + JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` } // OIDCProviderConfig describes the configuration of an OIDC provider. diff --git a/generated/1.19/apis/config/v1alpha1/zz_generated.deepcopy.go b/generated/1.19/apis/config/v1alpha1/zz_generated.deepcopy.go index 262992cb..e2522c24 100644 --- a/generated/1.19/apis/config/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.19/apis/config/v1alpha1/zz_generated.deepcopy.go @@ -216,6 +216,7 @@ func (in *OIDCProviderConfigStatus) DeepCopyInto(out *OIDCProviderConfigStatus) in, out := &in.LastUpdateTime, &out.LastUpdateTime *out = (*in).DeepCopy() } + out.JWKSSecret = in.JWKSSecret return } diff --git a/generated/1.19/client/openapi/zz_generated.openapi.go b/generated/1.19/client/openapi/zz_generated.openapi.go index 277a97cb..dd6e0519 100644 --- a/generated/1.19/client/openapi/zz_generated.openapi.go +++ b/generated/1.19/client/openapi/zz_generated.openapi.go @@ -432,11 +432,17 @@ func schema_119_apis_config_v1alpha1_OIDCProviderConfigStatus(ref common.Referen Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), }, }, + "jwksSecret": { + SchemaProps: spec.SchemaProps{ + Description: "JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist.", + Ref: ref("k8s.io/api/core/v1.LocalObjectReference"), + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, + "k8s.io/api/core/v1.LocalObjectReference", "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, } } diff --git a/generated/1.19/crds/config.pinniped.dev_oidcproviderconfigs.yaml b/generated/1.19/crds/config.pinniped.dev_oidcproviderconfigs.yaml index 062dca9f..727f20ec 100644 --- a/generated/1.19/crds/config.pinniped.dev_oidcproviderconfigs.yaml +++ b/generated/1.19/crds/config.pinniped.dev_oidcproviderconfigs.yaml @@ -55,6 +55,17 @@ spec: status: description: Status of the OIDC provider. properties: + jwksSecret: + description: JWKSSecret holds the name of the secret in which this + OIDC Provider's signing/verification keys are stored. If it is empty, + then the signing/verification keys are either unknown or they don't + exist. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object lastUpdateTime: description: LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior diff --git a/go.mod b/go.mod index a3f63b6b..6197d0e3 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( go.pinniped.dev/generated/1.19/client v0.0.0-00010101000000-000000000000 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6 + gopkg.in/square/go-jose.v2 v2.5.1 k8s.io/api v0.19.2 k8s.io/apimachinery v0.19.2 k8s.io/apiserver v0.19.2 diff --git a/go.sum b/go.sum index dae855b9..21f57d39 100644 --- a/go.sum +++ b/go.sum @@ -842,6 +842,8 @@ gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24 gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= gopkg.in/square/go-jose.v2 v2.2.2 h1:orlkJ3myw8CN1nVQHBFfloD+L3egixIa4FvUP6RosSA= gopkg.in/square/go-jose.v2 v2.2.2/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= +gopkg.in/square/go-jose.v2 v2.5.1 h1:7odma5RETjNHWJnR32wx8t+Io4djHE1PqxCFx3iiZ2w= +gopkg.in/square/go-jose.v2 v2.5.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74= diff --git a/internal/controller/supervisorconfig/jwks.go b/internal/controller/supervisorconfig/jwks.go new file mode 100644 index 00000000..f2b730ba --- /dev/null +++ b/internal/controller/supervisorconfig/jwks.go @@ -0,0 +1,332 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package supervisorconfig + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "fmt" + + "gopkg.in/square/go-jose.v2" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/util/retry" + "k8s.io/klog/v2" + + configv1alpha1 "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" + pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned" + configinformers "go.pinniped.dev/generated/1.19/client/informers/externalversions/config/v1alpha1" + pinnipedcontroller "go.pinniped.dev/internal/controller" + "go.pinniped.dev/internal/controllerlib" +) + +// These constants are the keys in an OPC's Secret's Data map. +const ( + // activeJWKKey points to the current private key used for signing tokens. + // + // Note! The value for this key will contain private key material! + activeJWKKey = "activeJWK" + // validJWKSKey points to the current JWKS used to verify tokens. + // + // Note! The value for this key will contain only public key material! + validJWKSKey = "validJWKS" +) + +const ( + opcKind = "OIDCProviderConfig" +) + +// 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 { + pinnipedClient pinnipedclientset.Interface + kubeClient kubernetes.Interface + opcInformer configinformers.OIDCProviderConfigInformer + secretInformer corev1informers.SecretInformer +} + +// NewJWKSController returns a controllerlib.Controller that ensures an OPC has a corresponding +// Secret that contains a valid active JWK and JWKS. +func NewJWKSController( + kubeClient kubernetes.Interface, + pinnipedClient pinnipedclientset.Interface, + secretInformer corev1informers.SecretInformer, + opcInformer configinformers.OIDCProviderConfigInformer, + withInformer pinnipedcontroller.WithInformerOptionFunc, +) controllerlib.Controller { + return controllerlib.New( + controllerlib.Config{ + Name: "JWKSController", + Syncer: &jwksController{ + kubeClient: kubeClient, + pinnipedClient: pinnipedClient, + secretInformer: secretInformer, + opcInformer: opcInformer, + }, + }, + // We want to be notified when a OPC's secret gets updated or deleted. When this happens, we + // should get notified via the corresponding OPC key. + withInformer( + secretInformer, + controllerlib.FilterFuncs{ + ParentFunc: func(obj metav1.Object) controllerlib.Key { + if isOPCControllee(obj) { + controller := metav1.GetControllerOf(obj) + return controllerlib.Key{ + Name: controller.Name, + Namespace: obj.GetNamespace(), + } + } + return controllerlib.Key{} + }, + AddFunc: func(obj metav1.Object) bool { + return false + }, + UpdateFunc: func(oldObj, newObj metav1.Object) bool { + return isOPCControllee(oldObj) || isOPCControllee(newObj) + }, + DeleteFunc: func(obj metav1.Object) bool { + return isOPCControllee(obj) + }, + }, + controllerlib.InformerOption{}, + ), + // We want to be notified when anything happens to an OPC. + withInformer( + opcInformer, + pinnipedcontroller.NoOpFilter(), + controllerlib.InformerOption{}, + ), + ) +} + +// Sync implements controllerlib.Syncer. +func (c *jwksController) Sync(ctx controllerlib.Context) error { + opc, err := c.opcInformer.Lister().OIDCProviderConfigs(ctx.Key.Namespace).Get(ctx.Key.Name) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return fmt.Errorf( + "failed to get %s/%s OIDCProviderConfig: %w", + ctx.Key.Namespace, + ctx.Key.Name, + err, + ) + } + + if notFound { + // The corresponding secret to this OPC should have been garbage collected since it should have + // had this OPC as its owner. + klog.InfoS( + "oidcproviderconfig deleted", + "oidcproviderconfig", + klog.KRef(ctx.Key.Namespace, ctx.Key.Name), + ) + 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) + } + } + + 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) + } + } + + // 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. + newOPC := opc.DeepCopy() + newOPC.Status.JWKSSecret.Name = secret.Name + if err := c.updateOPC(ctx.Context, newOPC); err != nil { + return fmt.Errorf("cannot update opc: %w", err) + } + klog.InfoS("updated oidcproviderconfig", "oidcproviderconfig", klog.KObj(newOPC)) + + return 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 + // connection to KMS, etc). + // + // For now, we just generate an new RSA keypair and put that in the secret. + + privateKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + return nil, fmt.Errorf("cannot generate key: %w", err) + } + + jwk := jose.JSONWebKey{ + Key: privateKey, + KeyID: "some-key", + Algorithm: "RS256", + Use: "sig", + } + jwkData, err := json.Marshal(jwk) + if err != nil { + return nil, fmt.Errorf("cannot marshal jwk: %w", err) + } + + jwks := jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{jwk}, + } + jwksData, err := json.Marshal(jwks) + if err != nil { + return nil, fmt.Errorf("cannot marshal jwks: %w", err) + } + + s := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: opc.Name + "-jwks", + Namespace: opc.Namespace, + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(opc, schema.GroupVersionKind{ + Group: configv1alpha1.SchemeGroupVersion.Group, + Version: configv1alpha1.SchemeGroupVersion.Version, + Kind: opcKind, + }), + }, + // TODO: custom labels. + }, + Data: map[string][]byte{ + activeJWKKey: jwkData, + validJWKSKey: jwksData, + }, + } + + return &s, nil +} + +func (c *jwksController) createOrUpdateSecret( + ctx context.Context, + newSecret *corev1.Secret, +) error { + secretClient := c.kubeClient.CoreV1().Secrets(newSecret.Namespace) + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + oldSecret, err := secretClient.Get(ctx, newSecret.Name, metav1.GetOptions{}) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return fmt.Errorf("cannot get secret: %w", err) + } + + if notFound { + // New secret doesn't exist, so create it. + _, err := secretClient.Create(ctx, newSecret, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("cannot create secret: %w", err) + } + return nil + } + + // New secret already exists, so ensure it is up to date. + + if isValid(oldSecret) { + // If the secret already has valid JWK's, then we are good to go and we don't need an update. + return nil + } + + oldSecret.Data = newSecret.Data + _, err = secretClient.Update(ctx, oldSecret, metav1.UpdateOptions{}) + return err + }) +} + +func (c *jwksController) updateOPC( + ctx context.Context, + newOPC *configv1alpha1.OIDCProviderConfig, +) error { + 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 newOPC.Status.JWKSSecret.Name == oldOPC.Status.JWKSSecret.Name { + // If the existing OPC is up to date, we don't need to update it. + return nil + } + + oldOPC.Status.JWKSSecret.Name = newOPC.Status.JWKSSecret.Name + _, err = opcClient.Update(ctx, oldOPC, metav1.UpdateOptions{}) + return err + }) +} + +// 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 +} + +// isValid returns whether the provided secret contains a valid active JWK and verification JWKS. +func isValid(secret *corev1.Secret) bool { + jwkData, ok := secret.Data[activeJWKKey] + if !ok { + klog.InfoS("secret does not contain active jwk") + return false + } + + var activeJWK jose.JSONWebKey + if err := json.Unmarshal(jwkData, &activeJWK); err != nil { + klog.InfoS("cannot unmarshal active jwk", "err", err) + return false + } + + if !activeJWK.Valid() { + klog.InfoS("active jwk is not valid", "keyid", activeJWK.KeyID) + return false + } + + jwksData, ok := secret.Data[validJWKSKey] + if !ok { + klog.InfoS("secret does not contain valid jwks") + } + + var validJWKS jose.JSONWebKeySet + if err := json.Unmarshal(jwksData, &validJWKS); err != nil { + klog.InfoS("cannot unmarshal valid jwks", "err", err) + return false + } + + foundActiveJWK := false + for _, validJWK := range validJWKS.Keys { + if !validJWK.Valid() { + klog.InfoS("jwks key is not valid", "keyid", validJWK.KeyID) + return false + } + if validJWK.KeyID == activeJWK.KeyID { + foundActiveJWK = true + } + } + + if !foundActiveJWK { + klog.InfoS("did not find active jwk in valid jwks", "keyid", activeJWK.KeyID) + return false + } + + return true +} diff --git a/internal/controller/supervisorconfig/jwks_test.go b/internal/controller/supervisorconfig/jwks_test.go new file mode 100644 index 00000000..7770a4e2 --- /dev/null +++ b/internal/controller/supervisorconfig/jwks_test.go @@ -0,0 +1,9 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package supervisorconfig + +import "testing" + +func TestJWKSControllerSync(t *testing.T) { +} From c030551af08a98c09010837e1ad67ef8a5b6a372 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Wed, 14 Oct 2020 16:41:16 -0400 Subject: [PATCH 02/10] supervisor-generate-key: unit and integration tests Signed-off-by: Andrew Keesler --- go.sum | 2 + hack/lib/tilt/Tiltfile | 2 +- internal/controller/supervisorconfig/jwks.go | 102 ++- .../controller/supervisorconfig/jwks_test.go | 692 +++++++++++++++++- .../supervisorconfig/testdata/good-jwk.json | 14 + .../supervisorconfig/testdata/good-jwks.json | 12 + .../testdata/good-rsa-key.pem | 27 + .../testdata/invalid-key-jwk.json | 8 + .../testdata/invalid-key-jwks.json | 12 + .../testdata/missing-active-jwks.json | 12 + .../supervisorconfig/testdata/not-json.txt | 1 + .../testdata/private-jwks.json | 18 + .../supervisorconfig/testdata/public-jwk.json | 8 + test/integration/supervisor_keys_test.go | 96 +++ test/library/client.go | 53 ++ 15 files changed, 1025 insertions(+), 34 deletions(-) create mode 100644 internal/controller/supervisorconfig/testdata/good-jwk.json create mode 100644 internal/controller/supervisorconfig/testdata/good-jwks.json create mode 100644 internal/controller/supervisorconfig/testdata/good-rsa-key.pem create mode 100644 internal/controller/supervisorconfig/testdata/invalid-key-jwk.json create mode 100644 internal/controller/supervisorconfig/testdata/invalid-key-jwks.json create mode 100644 internal/controller/supervisorconfig/testdata/missing-active-jwks.json create mode 100644 internal/controller/supervisorconfig/testdata/not-json.txt create mode 100644 internal/controller/supervisorconfig/testdata/private-jwks.json create mode 100644 internal/controller/supervisorconfig/testdata/public-jwk.json create mode 100644 test/integration/supervisor_keys_test.go 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 +} From 87c7e9a55602d4df3e4d88a38f9594ba7261d14b Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Wed, 14 Oct 2020 19:58:43 -0400 Subject: [PATCH 03/10] hack/prepare-for-integration-tests.sh: default COLORTERM for when not set Signed-off-by: Andrew Keesler --- hack/prepare-for-integration-tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index f0eb92ca..5e0296fd 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -20,7 +20,7 @@ function tilt_mode() { function log_note() { GREEN='\033[0;32m' NC='\033[0m' - if [[ $COLORTERM =~ ^(truecolor|24bit)$ ]]; then + if [[ ${COLORTERM:-unknown} =~ ^(truecolor|24bit)$ ]]; then echo -e "${GREEN}$*${NC}" else echo "$*" @@ -30,7 +30,7 @@ function log_note() { function log_error() { RED='\033[0;31m' NC='\033[0m' - if [[ $COLORTERM =~ ^(truecolor|24bit)$ ]]; then + if [[ ${COLORTERM:-unknown} =~ ^(truecolor|24bit)$ ]]; then echo -e "🙁${RED} Error: $* ${NC}" else echo ":( Error: $*" From aef25163e2e6a7c0a28d9691eda6c56a8a648777 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 15 Oct 2020 07:50:53 -0400 Subject: [PATCH 04/10] Get rid of an extra dependency from c030551 I brought this over because I copied code from work in flight on another branch. But now the other branch doesn't use this package. No use bringing on another dependency if we can avoid it. Signed-off-by: Andrew Keesler --- go.mod | 1 - test/library/client.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/go.mod b/go.mod index eb66e921..6adfc17b 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,6 @@ require ( github.com/golangci/golangci-lint v1.31.0 github.com/google/go-cmp v0.5.2 github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4 - github.com/pkg/errors v0.9.1 github.com/sclevine/agouti v3.0.0+incompatible github.com/sclevine/spec v1.4.0 github.com/spf13/cobra v1.0.0 diff --git a/test/library/client.go b/test/library/client.go index 6ecc0ce8..7460526a 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -14,7 +14,6 @@ import ( "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" @@ -201,7 +200,7 @@ func CreateTestOIDCProvider(ctx context.Context, t *testing.T) *configv1alpha1.O 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.Errorf("could not generate random state: %w", err) } return fmt.Sprintf("http://test-issuer-%s.pinniped.dev", hex.EncodeToString(buf[:])), nil } From 31225ac7ae61960ff997250c54abcbd72f9ae23e Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 15 Oct 2020 09:09:49 -0400 Subject: [PATCH 05/10] test/integration: reuse CreateTestOIDCProvider helper Signed-off-by: Andrew Keesler --- internal/controller/supervisorconfig/jwks.go | 5 ++++ test/integration/supervisor_discovery_test.go | 25 +++++++++++-------- test/integration/supervisor_keys_test.go | 2 +- test/library/client.go | 19 +++++++++++--- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/internal/controller/supervisorconfig/jwks.go b/internal/controller/supervisorconfig/jwks.go index 0ba44b72..05f74491 100644 --- a/internal/controller/supervisorconfig/jwks.go +++ b/internal/controller/supervisorconfig/jwks.go @@ -142,6 +142,11 @@ func (c *jwksController) Sync(ctx controllerlib.Context) error { } if !secretNeedsUpdate { // Secret is up to date - we are good to go. + klog.InfoS( + "secret is up to date", + "oidcproviderconfig", + klog.KRef(ctx.Key.Namespace, ctx.Key.Name), + ) return nil } diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 77dcaebf..ce362d63 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -63,14 +63,14 @@ func TestSupervisorOIDCDiscovery(t *testing.T) { badIssuer := fmt.Sprintf("http://%s/badIssuer?cannot-use=queries", env.SupervisorAddress) // When OIDCProviderConfig are created in sequence they each cause a discovery endpoint to appear only for as long as the OIDCProviderConfig exists. - config1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer1, "from-integration-test1") + config1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer1, client) requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, config1, client, ns, issuer1) - config2 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer2, "from-integration-test2") + config2 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer2, client) requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, config2, client, ns, issuer2) // When multiple OIDCProviderConfigs exist at the same time they each serve a unique discovery endpoint. - config3 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer3, "from-integration-test3") - config4 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer4, "from-integration-test4") + config3 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer3, client) + config4 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer4, client) requireWellKnownEndpointIsWorking(t, issuer3) // discovery for issuer3 is still working after issuer4 started working // When they are deleted they stop serving discovery endpoints. @@ -78,8 +78,8 @@ func TestSupervisorOIDCDiscovery(t *testing.T) { requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, config4, client, ns, issuer4) // When the same issuer is added twice, both issuers are marked as duplicates, and neither provider is serving. - config5Duplicate1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer5, "from-integration-test5") - config5Duplicate2 := createOIDCProviderConfig(t, "from-integration-test5-duplicate", client, ns, issuer5) + config5Duplicate1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer5, client) + config5Duplicate2 := library.CreateTestOIDCProvider(ctx, t, issuer5) requireStatus(t, client, ns, config5Duplicate1.Name, v1alpha1.DuplicateOIDCProviderStatus) requireStatus(t, client, ns, config5Duplicate2.Name, v1alpha1.DuplicateOIDCProviderStatus) requireDiscoveryEndpointIsNotFound(t, issuer5) @@ -93,7 +93,7 @@ func TestSupervisorOIDCDiscovery(t *testing.T) { requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, config5Duplicate2, client, ns, issuer5) // When we create a provider with an invalid issuer, the status is set to invalid. - badConfig := createOIDCProviderConfig(t, "from-integration-test6", client, ns, badIssuer) + badConfig := library.CreateTestOIDCProvider(ctx, t, badIssuer) requireStatus(t, client, ns, badConfig.Name, v1alpha1.InvalidOIDCProviderStatus) requireDiscoveryEndpointIsNotFound(t, badIssuer) } @@ -122,11 +122,16 @@ func requireDiscoveryEndpointIsNotFound(t *testing.T, issuerName string) { require.NoError(t, err) } -func requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t *testing.T, client pinnipedclientset.Interface, ns string, issuerName string, oidcProviderConfigName string) *v1alpha1.OIDCProviderConfig { +func requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear( + ctx context.Context, + t *testing.T, + issuerName string, + client pinnipedclientset.Interface, +) *v1alpha1.OIDCProviderConfig { t.Helper() - newOIDCProviderConfig := createOIDCProviderConfig(t, oidcProviderConfigName, client, ns, issuerName) + newOIDCProviderConfig := library.CreateTestOIDCProvider(ctx, t, issuerName) requireWellKnownEndpointIsWorking(t, issuerName) - requireStatus(t, client, ns, oidcProviderConfigName, v1alpha1.SuccessOIDCProviderStatus) + requireStatus(t, client, newOIDCProviderConfig.Namespace, newOIDCProviderConfig.Name, v1alpha1.SuccessOIDCProviderStatus) return newOIDCProviderConfig } diff --git a/test/integration/supervisor_keys_test.go b/test/integration/supervisor_keys_test.go index 09c720ec..feb7f50c 100644 --- a/test/integration/supervisor_keys_test.go +++ b/test/integration/supervisor_keys_test.go @@ -28,7 +28,7 @@ func TestSupervisorOIDCKeys(t *testing.T) { // Create our OPC under test. // TODO: maybe use this in other supervisor test? - opc := library.CreateTestOIDCProvider(ctx, t) + opc := library.CreateTestOIDCProvider(ctx, t, "") // Ensure a secret is created with the OPC's JWKS. var updatedOPC *configv1alpha1.OIDCProviderConfig diff --git a/test/library/client.go b/test/library/client.go index 7460526a..f12219da 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -161,15 +162,21 @@ func CreateTestWebhookIDP(ctx context.Context, t *testing.T) corev1.TypedLocalOb // 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 { +// +// If the provided issuer is not the empty string, then it will be used for the +// OIDCProviderConfig.Spec.Issuer field. Else, a random issuer will be generated. +func CreateTestOIDCProvider(ctx context.Context, t *testing.T, issuer string) *configv1alpha1.OIDCProviderConfig { t.Helper() testEnv := IntegrationEnv(t) createContext, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() - issuer, err := randomIssuer() - require.NoError(t, err) + if issuer == "" { + var err error + issuer, err = randomIssuer() + require.NoError(t, err) + } opcs := NewPinnipedClientset(t).ConfigV1alpha1().OIDCProviderConfigs(testEnv.SupervisorNamespace) opc, err := opcs.Create(createContext, &configv1alpha1.OIDCProviderConfig{ @@ -191,7 +198,11 @@ func CreateTestOIDCProvider(ctx context.Context, t *testing.T) *configv1alpha1.O 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) + notFound := k8serrors.IsNotFound(err) + // It's okay if it is not found, because it might have been deleted by another part of this test. + if !notFound { + require.NoErrorf(t, err, "could not cleanup test OIDCProviderConfig %s/%s", opc.Namespace, opc.Name) + } }) return opc From 1b99983441407f80736945c435713e8be844fbb1 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 15 Oct 2020 09:19:00 -0400 Subject: [PATCH 06/10] apis: fix indentation in Go type Signed-off-by: Andrew Keesler --- apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl | 10 +++++----- .../apis/config/v1alpha1/types_oidcproviderconfig.go | 10 +++++----- .../apis/config/v1alpha1/types_oidcproviderconfig.go | 10 +++++----- .../apis/config/v1alpha1/types_oidcproviderconfig.go | 10 +++++----- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl b/apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl index b8b34560..8ecf6c83 100644 --- a/apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl +++ b/apis/config/v1alpha1/types_oidcproviderconfig.go.tmpl @@ -12,9 +12,9 @@ import ( type OIDCProviderStatus string const ( - SuccessOIDCProviderStatus = OIDCProviderStatus("Success") + SuccessOIDCProviderStatus = OIDCProviderStatus("Success") DuplicateOIDCProviderStatus = OIDCProviderStatus("Duplicate") - InvalidOIDCProviderStatus = OIDCProviderStatus("Invalid") + InvalidOIDCProviderStatus = OIDCProviderStatus("Invalid") ) // OIDCProviderConfigSpec is a struct that describes an OIDC Provider. @@ -48,9 +48,9 @@ type OIDCProviderConfigStatus struct { // +optional LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` - // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys - // are stored. If it is empty, then the signing/verification keys are either unknown or they don't - // exist. + // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys + // are stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. // +optional JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` } diff --git a/generated/1.17/apis/config/v1alpha1/types_oidcproviderconfig.go b/generated/1.17/apis/config/v1alpha1/types_oidcproviderconfig.go index b8b34560..8ecf6c83 100644 --- a/generated/1.17/apis/config/v1alpha1/types_oidcproviderconfig.go +++ b/generated/1.17/apis/config/v1alpha1/types_oidcproviderconfig.go @@ -12,9 +12,9 @@ import ( type OIDCProviderStatus string const ( - SuccessOIDCProviderStatus = OIDCProviderStatus("Success") + SuccessOIDCProviderStatus = OIDCProviderStatus("Success") DuplicateOIDCProviderStatus = OIDCProviderStatus("Duplicate") - InvalidOIDCProviderStatus = OIDCProviderStatus("Invalid") + InvalidOIDCProviderStatus = OIDCProviderStatus("Invalid") ) // OIDCProviderConfigSpec is a struct that describes an OIDC Provider. @@ -48,9 +48,9 @@ type OIDCProviderConfigStatus struct { // +optional LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` - // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys - // are stored. If it is empty, then the signing/verification keys are either unknown or they don't - // exist. + // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys + // are stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. // +optional JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` } diff --git a/generated/1.18/apis/config/v1alpha1/types_oidcproviderconfig.go b/generated/1.18/apis/config/v1alpha1/types_oidcproviderconfig.go index b8b34560..8ecf6c83 100644 --- a/generated/1.18/apis/config/v1alpha1/types_oidcproviderconfig.go +++ b/generated/1.18/apis/config/v1alpha1/types_oidcproviderconfig.go @@ -12,9 +12,9 @@ import ( type OIDCProviderStatus string const ( - SuccessOIDCProviderStatus = OIDCProviderStatus("Success") + SuccessOIDCProviderStatus = OIDCProviderStatus("Success") DuplicateOIDCProviderStatus = OIDCProviderStatus("Duplicate") - InvalidOIDCProviderStatus = OIDCProviderStatus("Invalid") + InvalidOIDCProviderStatus = OIDCProviderStatus("Invalid") ) // OIDCProviderConfigSpec is a struct that describes an OIDC Provider. @@ -48,9 +48,9 @@ type OIDCProviderConfigStatus struct { // +optional LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` - // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys - // are stored. If it is empty, then the signing/verification keys are either unknown or they don't - // exist. + // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys + // are stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. // +optional JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` } diff --git a/generated/1.19/apis/config/v1alpha1/types_oidcproviderconfig.go b/generated/1.19/apis/config/v1alpha1/types_oidcproviderconfig.go index b8b34560..8ecf6c83 100644 --- a/generated/1.19/apis/config/v1alpha1/types_oidcproviderconfig.go +++ b/generated/1.19/apis/config/v1alpha1/types_oidcproviderconfig.go @@ -12,9 +12,9 @@ import ( type OIDCProviderStatus string const ( - SuccessOIDCProviderStatus = OIDCProviderStatus("Success") + SuccessOIDCProviderStatus = OIDCProviderStatus("Success") DuplicateOIDCProviderStatus = OIDCProviderStatus("Duplicate") - InvalidOIDCProviderStatus = OIDCProviderStatus("Invalid") + InvalidOIDCProviderStatus = OIDCProviderStatus("Invalid") ) // OIDCProviderConfigSpec is a struct that describes an OIDC Provider. @@ -48,9 +48,9 @@ type OIDCProviderConfigStatus struct { // +optional LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` - // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys - // are stored. If it is empty, then the signing/verification keys are either unknown or they don't - // exist. + // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys + // are stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. // +optional JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` } From a5abe9ca3eece194776bf8fe44d9c12192b09058 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 15 Oct 2020 09:20:09 -0400 Subject: [PATCH 07/10] hack/lib/tilt: fix deployment change leftover from c030551a Signed-off-by: Andrew Keesler --- hack/lib/tilt/Tiltfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/lib/tilt/Tiltfile b/hack/lib/tilt/Tiltfile index 9859835a..4f96c775 100644 --- a/hack/lib/tilt/Tiltfile +++ b/hack/lib/tilt/Tiltfile @@ -92,7 +92,7 @@ k8s_yaml(local([ '--data-value', 'namespace=supervisor', '--data-value', 'image_repo=image/supervisor', '--data-value', 'image_tag=tilt-dev', - '--data-value-yaml', 'replicas=2', + '--data-value-yaml', 'replicas=1', '--data-value-yaml', 'service_nodeport_port=31234', ])) # Tell tilt to watch all of those files for changes. From fbcce700dc67b2326271d52d34c0d5a334c2af55 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 15 Oct 2020 09:22:17 -0400 Subject: [PATCH 08/10] Fix whitespace/spelling nits in JWKS controller Signed-off-by: Andrew Keesler --- internal/controller/supervisorconfig/jwks.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/controller/supervisorconfig/jwks.go b/internal/controller/supervisorconfig/jwks.go index 05f74491..24fa67ff 100644 --- a/internal/controller/supervisorconfig/jwks.go +++ b/internal/controller/supervisorconfig/jwks.go @@ -52,7 +52,7 @@ 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 +// jwkController holds the fields necessary for the JWKS controller to communicate with OPC's and // secrets, both via a cache and via the API. type jwksController struct { pinnipedClient pinnipedclientset.Interface @@ -160,7 +160,6 @@ func (c *jwksController) Sync(ctx controllerlib.Context) error { 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. From 5a0dab768ff3008111ae20dc7d02de5f40a51385 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 15 Oct 2020 09:26:15 -0400 Subject: [PATCH 09/10] test/integration: remove unused function (see 31225ac7a) Signed-off-by: Andrew Keesler --- test/integration/supervisor_discovery_test.go | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index ce362d63..9b983fe2 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" @@ -197,43 +196,6 @@ func requireWellKnownEndpointIsWorking(t *testing.T, issuerName string) { require.JSONEq(t, expectedJSON, string(responseBody)) } -func createOIDCProviderConfig(t *testing.T, oidcProviderConfigName string, client pinnipedclientset.Interface, ns string, issuerName string) *v1alpha1.OIDCProviderConfig { - t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) - defer cancel() - - newOIDCProviderConfig := v1alpha1.OIDCProviderConfig{ - TypeMeta: metav1.TypeMeta{ - Kind: "OIDCProviderConfig", - APIVersion: v1alpha1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: oidcProviderConfigName, - Namespace: ns, - }, - Spec: v1alpha1.OIDCProviderConfigSpec{ - Issuer: issuerName, - }, - } - createdOIDCProviderConfig, err := client.ConfigV1alpha1().OIDCProviderConfigs(ns).Create(ctx, &newOIDCProviderConfig, metav1.CreateOptions{}) - require.NoError(t, err) - - // When this test has finished, be sure to clean up the new OIDCProviderConfig. - t.Cleanup(func() { - cleanupCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) - defer cancel() - - err = client.ConfigV1alpha1().OIDCProviderConfigs(ns).Delete(cleanupCtx, newOIDCProviderConfig.Name, metav1.DeleteOptions{}) - notFound := k8serrors.IsNotFound(err) - // It's okay if it is not found, because it might have been deleted by another part of this test. - if !notFound { - require.NoError(t, err) - } - }) - - return createdOIDCProviderConfig -} - func requireDelete(t *testing.T, client pinnipedclientset.Interface, ns, name string) { t.Helper() ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) From e05213f9dd55a8bc76e16dd825c925c3a8421364 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 15 Oct 2020 11:33:08 -0400 Subject: [PATCH 10/10] supervisor-generate-key: use EC keys intead of RSA EC keys are smaller and take less time to generate. Our integration tests were super flakey because generating an RSA key would take up to 10 seconds *gasp*. The main token verifier that we care about is Kubernetes, which supports P256, so hopefully it won't be that much of an issue that our default signing key type is EC. The OIDC spec seems kinda squirmy when it comes to using non-RSA signing algorithms... Signed-off-by: Andrew Keesler --- internal/controller/supervisorconfig/jwks.go | 17 ++++++------ .../controller/supervisorconfig/jwks_test.go | 12 ++++----- .../supervisorconfig/testdata/good-ec-key.pem | 5 ++++ .../supervisorconfig/testdata/good-jwk.json | 18 +++++-------- .../supervisorconfig/testdata/good-jwks.json | 11 ++++---- .../testdata/good-rsa-key.pem | 27 ------------------- .../testdata/invalid-key-jwk.json | 12 +++++---- .../testdata/invalid-key-jwks.json | 11 ++++---- .../testdata/missing-active-jwks.json | 9 ++++--- .../testdata/private-jwks.json | 18 +++++-------- .../supervisorconfig/testdata/public-jwk.json | 11 ++++---- test/integration/supervisor_keys_test.go | 1 - 12 files changed, 64 insertions(+), 88 deletions(-) create mode 100644 internal/controller/supervisorconfig/testdata/good-ec-key.pem delete mode 100644 internal/controller/supervisorconfig/testdata/good-rsa-key.pem diff --git a/internal/controller/supervisorconfig/jwks.go b/internal/controller/supervisorconfig/jwks.go index 24fa67ff..a507df90 100644 --- a/internal/controller/supervisorconfig/jwks.go +++ b/internal/controller/supervisorconfig/jwks.go @@ -5,8 +5,9 @@ package supervisorconfig import ( "context" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" "encoding/json" "fmt" "io" @@ -44,12 +45,12 @@ const ( opcKind = "OIDCProviderConfig" ) -// generateKey is stubbed out for the purpose of testing. The default behavior is to generate an RSA key. +// generateKey is stubbed out for the purpose of testing. The default behavior is to generate an EC key. //nolint:gochecknoglobals -var generateKey func(r io.Reader, bits int) (interface{}, error) = generateRSAKey +var generateKey func(r io.Reader) (interface{}, error) = generateECKey -func generateRSAKey(r io.Reader, bits int) (interface{}, error) { - return rsa.GenerateKey(r, bits) +func generateECKey(r io.Reader) (interface{}, error) { + return ecdsa.GenerateKey(elliptic.P256(), r) } // jwkController holds the fields necessary for the JWKS controller to communicate with OPC's and @@ -205,15 +206,15 @@ func (c *jwksController) generateSecret(opc *configv1alpha1.OIDCProviderConfig) // // For now, we just generate an new RSA keypair and put that in the secret. - key, err := generateKey(rand.Reader, 4096) + key, err := generateKey(rand.Reader) if err != nil { return nil, fmt.Errorf("cannot generate key: %w", err) } jwk := jose.JSONWebKey{ Key: key, - KeyID: "some-key", - Algorithm: "RS256", + KeyID: "pinniped-supervisor-key", + Algorithm: "ES256", Use: "sig", } jwkData, err := json.Marshal(jwk) diff --git a/internal/controller/supervisorconfig/jwks_test.go b/internal/controller/supervisorconfig/jwks_test.go index 30298db4..ecab24ad 100644 --- a/internal/controller/supervisorconfig/jwks_test.go +++ b/internal/controller/supervisorconfig/jwks_test.go @@ -227,11 +227,11 @@ func TestJWKSControllerSync(t *testing.T) { const namespace = "tuna-namespace" - goodRSAKeyPEM, err := ioutil.ReadFile("testdata/good-rsa-key.pem") + goodKeyPEM, err := ioutil.ReadFile("testdata/good-ec-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) + block, _ := pem.Decode(goodKeyPEM) + require.NotNil(t, block, "expected block to be non-nil...is goodKeyPEM a valid PEM?") + goodKey, err := x509.ParseECPrivateKey(block.Bytes) require.NoError(t, err) opcGVR := schema.GroupVersionResource{ @@ -610,9 +610,9 @@ func TestJWKSControllerSync(t *testing.T) { 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) { + generateKey = func(_ io.Reader) (interface{}, error) { generateKeyCount++ - return goodRSAKey, test.generateKeyErr + return goodKey, test.generateKeyErr } ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) diff --git a/internal/controller/supervisorconfig/testdata/good-ec-key.pem b/internal/controller/supervisorconfig/testdata/good-ec-key.pem new file mode 100644 index 00000000..211202f4 --- /dev/null +++ b/internal/controller/supervisorconfig/testdata/good-ec-key.pem @@ -0,0 +1,5 @@ +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEINR2PAduYBO64CaDT4vLoMnn8y4UX5VTFdOA7wUQF0n/oAoGCCqGSM49 +AwEHoUQDQgAEawmmj6CIMhSoJyfsqH7sekbTeY72GGPLEy16tPWVz2UVwyHTq5ct +qr1vYw6LGUtWJ1STJw7W7sgc6StOLs3RrA== +-----END EC PRIVATE KEY----- diff --git a/internal/controller/supervisorconfig/testdata/good-jwk.json b/internal/controller/supervisorconfig/testdata/good-jwk.json index 20cda4f3..89e4be66 100644 --- a/internal/controller/supervisorconfig/testdata/good-jwk.json +++ b/internal/controller/supervisorconfig/testdata/good-jwk.json @@ -1,14 +1,10 @@ { "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" + "kty": "EC", + "kid": "pinniped-supervisor-key", + "crv": "P-256", + "alg": "ES256", + "x": "awmmj6CIMhSoJyfsqH7sekbTeY72GGPLEy16tPWVz2U", + "y": "FcMh06uXLaq9b2MOixlLVidUkycO1u7IHOkrTi7N0aw", + "d": "1HY8B25gE7rgJoNPi8ugyefzLhRflVMV04DvBRAXSf8" } diff --git a/internal/controller/supervisorconfig/testdata/good-jwks.json b/internal/controller/supervisorconfig/testdata/good-jwks.json index ddb4fecb..b099245e 100644 --- a/internal/controller/supervisorconfig/testdata/good-jwks.json +++ b/internal/controller/supervisorconfig/testdata/good-jwks.json @@ -2,11 +2,12 @@ "keys": [ { "use": "sig", - "kty": "RSA", - "kid": "some-key", - "alg": "RS256", - "n": "z6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo2XshthG21-L82rmxUQ23-1XTkBSBK5iZl3Q_liHt1MLjZrpjuRc0CKMDcrExAMX6duicFVlhkIakIeupp-PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa-c2GXuEuX572CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnAdxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B_Q0g3vGyFNVf0MM4LX3OSre4yVlphZOW3YeLIeBq_4KmgutD0AZHzCF18KUjJgOv9w", - "e": "AQAB" + "kty": "EC", + "kid": "pinniped-supervisor-key", + "crv": "P-256", + "alg": "ES256", + "x": "awmmj6CIMhSoJyfsqH7sekbTeY72GGPLEy16tPWVz2U", + "y": "FcMh06uXLaq9b2MOixlLVidUkycO1u7IHOkrTi7N0aw" } ] } diff --git a/internal/controller/supervisorconfig/testdata/good-rsa-key.pem b/internal/controller/supervisorconfig/testdata/good-rsa-key.pem deleted file mode 100644 index 459761eb..00000000 --- a/internal/controller/supervisorconfig/testdata/good-rsa-key.pem +++ /dev/null @@ -1,27 +0,0 @@ ------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 index 4c51e5c9..cee9bfb9 100644 --- a/internal/controller/supervisorconfig/testdata/invalid-key-jwk.json +++ b/internal/controller/supervisorconfig/testdata/invalid-key-jwk.json @@ -1,8 +1,10 @@ { "use": "sig", - "kty": "RSA", - "kid": "some-key", - "alg": "RS256", - "n": "z6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo2XshthG21-L82rmxUQ23-1XTkBSBK5iZl3Q_liHt1MLjZrpjuRc0CKMDcrExAMX6duicFVlhkIakIeupp-PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa-c2GXuEuX572CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnAdxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B_Q0g3vGyFNVf0MM4LX3OSre4yVlphZOW3YeLIeBq_4KmgutD0AZHzCF18KUjJgOv9w", - "e": "0" + "kty": "EC", + "kid": "pinniped-supervisor-key", + "crv": "P-256", + "alg": "ES256", + "x": "0", + "y": "FcMh06uXLaq9b2MOixlLVidUkycO1u7IHOkrTi7N0aw", + "d": "1HY8B25gE7rgJoNPi8ugyefzLhRflVMV04DvBRAXSf8" } diff --git a/internal/controller/supervisorconfig/testdata/invalid-key-jwks.json b/internal/controller/supervisorconfig/testdata/invalid-key-jwks.json index b5eef40e..cff683f2 100644 --- a/internal/controller/supervisorconfig/testdata/invalid-key-jwks.json +++ b/internal/controller/supervisorconfig/testdata/invalid-key-jwks.json @@ -2,11 +2,12 @@ "keys": [ { "use": "sig", - "kty": "RSA", - "kid": "some-key", - "alg": "RS256", - "n": "0", - "e": "AQAB" + "kty": "EC", + "kid": "pinniped-supervisor-key", + "crv": "P-256", + "alg": "ES256", + "x": "awmmj6CIMhSoJyfsqH7sekbTeY72GGPLEy16tPWVz2U", + "y": "0" } ] } diff --git a/internal/controller/supervisorconfig/testdata/missing-active-jwks.json b/internal/controller/supervisorconfig/testdata/missing-active-jwks.json index 08e36e4f..c6e77e6f 100644 --- a/internal/controller/supervisorconfig/testdata/missing-active-jwks.json +++ b/internal/controller/supervisorconfig/testdata/missing-active-jwks.json @@ -2,11 +2,12 @@ "keys": [ { "use": "sig", - "kty": "RSA", + "kty": "EC", "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" + "crv": "P-256", + "alg": "ES256", + "x": "awmmj6CIMhSoJyfsqH7sekbTeY72GGPLEy16tPWVz2U", + "y": "0" } ] } diff --git a/internal/controller/supervisorconfig/testdata/private-jwks.json b/internal/controller/supervisorconfig/testdata/private-jwks.json index d72ce22b..b88f8fcc 100644 --- a/internal/controller/supervisorconfig/testdata/private-jwks.json +++ b/internal/controller/supervisorconfig/testdata/private-jwks.json @@ -2,17 +2,13 @@ "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" + "kty": "EC", + "kid": "pinniped-supervisor-key", + "crv": "P-256", + "alg": "ES256", + "x": "awmmj6CIMhSoJyfsqH7sekbTeY72GGPLEy16tPWVz2U", + "y": "FcMh06uXLaq9b2MOixlLVidUkycO1u7IHOkrTi7N0aw", + "d": "1HY8B25gE7rgJoNPi8ugyefzLhRflVMV04DvBRAXSf8" } ] } diff --git a/internal/controller/supervisorconfig/testdata/public-jwk.json b/internal/controller/supervisorconfig/testdata/public-jwk.json index 1b4b7a88..bd440e4e 100644 --- a/internal/controller/supervisorconfig/testdata/public-jwk.json +++ b/internal/controller/supervisorconfig/testdata/public-jwk.json @@ -1,8 +1,9 @@ { "use": "sig", - "kty": "RSA", - "kid": "some-key", - "alg": "RS256", - "n": "z6UWJvYJtVxXpvITGDdq9I2ln73zu7gH4RB4q7t5bKFPYAEo2XshthG21-L82rmxUQ23-1XTkBSBK5iZl3Q_liHt1MLjZrpjuRc0CKMDcrExAMX6duicFVlhkIakIeupp-PrlvLSp9ZNXuQ3z1eSKK51d2svHRSqJXdHBa-c2GXuEuX572CnV2oGO06L8f1Tt0yLT3HxzHMRbwntID9Rg2KJj0f5lBin2Kd4wJejHgBj8hnAdxe6nnDsFYqgUQu3Qao9edgwiX9EftzGlo9B_Q0g3vGyFNVf0MM4LX3OSre4yVlphZOW3YeLIeBq_4KmgutD0AZHzCF18KUjJgOv9w", - "e": "AQAB" + "kty": "EC", + "kid": "pinniped-supervisor-key", + "crv": "P-256", + "alg": "ES256", + "x": "awmmj6CIMhSoJyfsqH7sekbTeY72GGPLEy16tPWVz2U", + "y": "FcMh06uXLaq9b2MOixlLVidUkycO1u7IHOkrTi7N0aw" } diff --git a/test/integration/supervisor_keys_test.go b/test/integration/supervisor_keys_test.go index feb7f50c..b6f73e93 100644 --- a/test/integration/supervisor_keys_test.go +++ b/test/integration/supervisor_keys_test.go @@ -27,7 +27,6 @@ func TestSupervisorOIDCKeys(t *testing.T) { 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.