diff --git a/Dockerfile b/Dockerfile index aea42a74..7ddea4d4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -25,6 +25,7 @@ RUN mkdir out \ # Use a runtime image based on Debian slim FROM debian:10.6-slim +RUN apt-get update && apt-get install -y ca-certificates && rm -rf /var/lib/apt/lists/* # Copy the binaries from the build-env stage COPY --from=build-env /work/out/pinniped-concierge /usr/local/bin/pinniped-concierge diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index 18d69d58..d2bfc7f5 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -22,11 +22,13 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/component-base/logs" "k8s.io/klog/v2" + "k8s.io/klog/v2/klogr" pinnipedclientset "go.pinniped.dev/generated/1.19/client/supervisor/clientset/versioned" pinnipedinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/config/supervisor" "go.pinniped.dev/internal/controller/supervisorconfig" + "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatcher" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/downward" "go.pinniped.dev/internal/oidc/jwks" @@ -73,6 +75,7 @@ func startControllers( issuerManager *manager.Manager, dynamicJWKSProvider jwks.DynamicJWKSProvider, dynamicTLSCertProvider provider.DynamicTLSCertProvider, + dynamicUpstreamIDPProvider provider.DynamicUpstreamIDPProvider, kubeClient kubernetes.Interface, pinnipedClient pinnipedclientset.Interface, kubeInformers kubeinformers.SharedInformerFactory, @@ -120,7 +123,15 @@ func startControllers( controllerlib.WithInformer, ), singletonWorker, - ) + ). + WithController( + upstreamwatcher.New( + dynamicUpstreamIDPProvider, + pinnipedClient, + pinnipedInformers.IDP().V1alpha1().UpstreamOIDCProviders(), + kubeInformers.Core().V1().Secrets(), + klogr.New()), + singletonWorker) kubeInformers.Start(ctx.Done()) pinnipedInformers.Start(ctx.Done()) @@ -193,6 +204,7 @@ func run(serverInstallationNamespace string, cfg *supervisor.Config) error { oidProvidersManager, dynamicJWKSProvider, dynamicTLSCertProvider, + dynamicUpstreamIDPProvider, kubeClient, pinnipedClient, kubeInformers, diff --git a/hack/lib/tilt/supervisor.Dockerfile b/hack/lib/tilt/supervisor.Dockerfile index c47e2f23..468749ad 100644 --- a/hack/lib/tilt/supervisor.Dockerfile +++ b/hack/lib/tilt/supervisor.Dockerfile @@ -4,6 +4,8 @@ # Use a runtime image based on Debian slim FROM debian:10.6-slim +RUN apt-get update && apt-get install -y ca-certificates && rm -rf /var/lib/apt/lists/* + # Copy the binary which was built outside the container. COPY build/pinniped-supervisor /usr/local/bin/pinniped-supervisor diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go new file mode 100644 index 00000000..3b96043e --- /dev/null +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go @@ -0,0 +1,337 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package upstreamwatcher implements a controller that watches UpstreamOIDCProvider objects. +package upstreamwatcher + +import ( + "context" + "fmt" + "net/url" + "sort" + "strings" + "time" + + "github.com/coreos/go-oidc" + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/cache" + corev1informers "k8s.io/client-go/informers/core/v1" + + "go.pinniped.dev/generated/1.19/apis/supervisor/idp/v1alpha1" + pinnipedclientset "go.pinniped.dev/generated/1.19/client/supervisor/clientset/versioned" + idpinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions/idp/v1alpha1" + "go.pinniped.dev/internal/constable" + pinnipedcontroller "go.pinniped.dev/internal/controller" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/oidc/provider" +) + +const ( + // Setup for the name of our controller in logs. + controllerName = "upstream-observer" + + // Constants related to the client credentials Secret. + oidcClientSecretType = "secrets.pinniped.dev/oidc-client" + clientIDDataKey = "clientID" + clientSecretDataKey = "clientSecret" + + // Constants related to the OIDC provider discovery cache. These do not affect the cache of JWKS. + validatorCacheTTL = 15 * time.Minute + + // Constants related to conditions. + typeClientCredsValid = "ClientCredentialsValid" + typeOIDCDiscoverySucceeded = "OIDCDiscoverySucceeded" + reasonNotFound = "SecretNotFound" + reasonWrongType = "SecretWrongType" + reasonMissingKeys = "SecretMissingKeys" + reasonSuccess = "Success" + reasonUnreachable = "Unreachable" + reasonInvalidResponse = "InvalidResponse" + + // Errors that are generated by our reconcile process. + errFailureStatus = constable.Error("UpstreamOIDCProvider has a failing condition") +) + +// IDPCache is a thread safe cache that holds a list of validated upstream OIDC IDP configurations. +type IDPCache interface { + SetIDPList([]provider.UpstreamOIDCIdentityProvider) +} + +type controller struct { + cache IDPCache + log logr.Logger + client pinnipedclientset.Interface + providers idpinformers.UpstreamOIDCProviderInformer + secrets corev1informers.SecretInformer + validatorCache *cache.Expiring +} + +// New instantiates a new controllerlib.Controller which will populate the provided IDPCache. +func New( + idpCache IDPCache, + client pinnipedclientset.Interface, + providers idpinformers.UpstreamOIDCProviderInformer, + secrets corev1informers.SecretInformer, + log logr.Logger, +) controllerlib.Controller { + c := controller{ + cache: idpCache, + log: log.WithName(controllerName), + client: client, + providers: providers, + secrets: secrets, + validatorCache: cache.NewExpiring(), + } + filter := pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()) + return controllerlib.New( + controllerlib.Config{Name: controllerName, Syncer: &c}, + controllerlib.WithInformer(providers, filter, controllerlib.InformerOption{}), + controllerlib.WithInformer(secrets, filter, controllerlib.InformerOption{}), + ) +} + +// Sync implements controllerlib.Syncer. +func (c *controller) Sync(ctx controllerlib.Context) error { + actualUpstreams, err := c.providers.Lister().List(labels.Everything()) + if err != nil { + return fmt.Errorf("failed to list UpstreamOIDCProviders: %w", err) + } + + requeue := false + validatedUpstreams := make([]provider.UpstreamOIDCIdentityProvider, 0, len(actualUpstreams)) + for _, upstream := range actualUpstreams { + valid := c.validateUpstream(ctx, upstream) + if valid == nil { + requeue = true + } else { + validatedUpstreams = append(validatedUpstreams, *valid) + } + } + c.cache.SetIDPList(validatedUpstreams) + if requeue { + return controllerlib.ErrSyntheticRequeue + } + return nil +} + +// validateUpstream validates the provided v1alpha1.UpstreamOIDCProvider and returns the validated configuration as a +// provider.UpstreamOIDCIdentityProvider. As a side effect, it also updates the status of the v1alpha1.UpstreamOIDCProvider. +func (c *controller) validateUpstream(ctx controllerlib.Context, upstream *v1alpha1.UpstreamOIDCProvider) *provider.UpstreamOIDCIdentityProvider { + result := provider.UpstreamOIDCIdentityProvider{ + Name: upstream.Name, + Scopes: computeScopes(upstream.Spec.AuthorizationConfig.AdditionalScopes), + } + conditions := []*v1alpha1.Condition{ + c.validateSecret(upstream, &result), + c.validateIssuer(ctx.Context, upstream, &result), + } + c.updateStatus(ctx.Context, upstream, conditions) + + valid := true + log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) + for _, condition := range conditions { + if condition.Status == v1alpha1.ConditionFalse { + valid = false + log.WithValues( + "type", condition.Type, + "reason", condition.Reason, + "message", condition.Message, + ).Error(errFailureStatus, "found failing condition") + } + } + if valid { + return &result + } + return nil +} + +// validateSecret validates the .spec.client.secretName field and returns the appropriate ClientCredentialsValid condition. +func (c *controller) validateSecret(upstream *v1alpha1.UpstreamOIDCProvider, result *provider.UpstreamOIDCIdentityProvider) *v1alpha1.Condition { + secretName := upstream.Spec.Client.SecretName + + // Fetch the Secret from informer cache. + secret, err := c.secrets.Lister().Secrets(upstream.Namespace).Get(secretName) + if err != nil { + return &v1alpha1.Condition{ + Type: typeClientCredsValid, + Status: v1alpha1.ConditionFalse, + Reason: reasonNotFound, + Message: err.Error(), + } + } + + // Validate the secret .type field. + if secret.Type != oidcClientSecretType { + return &v1alpha1.Condition{ + Type: typeClientCredsValid, + Status: v1alpha1.ConditionFalse, + Reason: reasonWrongType, + Message: fmt.Sprintf("referenced Secret %q has wrong type %q (should be %q)", secretName, secret.Type, oidcClientSecretType), + } + } + + // Validate the secret .data field. + clientID := secret.Data[clientIDDataKey] + clientSecret := secret.Data[clientSecretDataKey] + if len(clientID) == 0 || len(clientSecret) == 0 { + return &v1alpha1.Condition{ + Type: typeClientCredsValid, + Status: v1alpha1.ConditionFalse, + Reason: reasonMissingKeys, + Message: fmt.Sprintf("referenced Secret %q is missing required keys %q", secretName, []string{clientIDDataKey, clientSecretDataKey}), + } + } + + // If everything is valid, update the result and set the condition to true. + result.ClientID = string(clientID) + return &v1alpha1.Condition{ + Type: typeClientCredsValid, + Status: v1alpha1.ConditionTrue, + Reason: reasonSuccess, + Message: "loaded client credentials", + } +} + +// validateIssuer validates the .spec.issuer field, performs OIDC discovery, and returns the appropriate OIDCDiscoverySucceeded condition. +func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.UpstreamOIDCProvider, result *provider.UpstreamOIDCIdentityProvider) *v1alpha1.Condition { + // Get the provider (from cache if possible). + var discoveredProvider *oidc.Provider + if cached, ok := c.validatorCache.Get(upstream.Spec.Issuer); ok { + discoveredProvider = cached.(*oidc.Provider) + } + + // If the provider does not exist in the cache, do a fresh discovery lookup and save to the cache. + if discoveredProvider == nil { + var err error + discoveredProvider, err = oidc.NewProvider(ctx, upstream.Spec.Issuer) + if err != nil { + err := fmt.Errorf("failed to perform OIDC discovery against %q: %w", upstream.Spec.Issuer, err) + return &v1alpha1.Condition{ + Type: typeOIDCDiscoverySucceeded, + Status: v1alpha1.ConditionFalse, + Reason: reasonUnreachable, + Message: strings.TrimSpace(err.Error()), + } + } + + // Update the cache with the newly discovered value. + c.validatorCache.Set(upstream.Spec.Issuer, discoveredProvider, validatorCacheTTL) + } + + // Parse out and validate the discovered authorize endpoint. + authURL, err := url.Parse(discoveredProvider.Endpoint().AuthURL) + if err != nil { + return &v1alpha1.Condition{ + Type: typeOIDCDiscoverySucceeded, + Status: v1alpha1.ConditionFalse, + Reason: reasonInvalidResponse, + Message: fmt.Sprintf("failed to parse authorization endpoint URL: %v", err), + } + } + if authURL.Scheme != "https" { + return &v1alpha1.Condition{ + Type: typeOIDCDiscoverySucceeded, + Status: v1alpha1.ConditionFalse, + Reason: reasonInvalidResponse, + Message: fmt.Sprintf(`authorization endpoint URL scheme must be "https", not %q`, authURL.Scheme), + } + } + + // If everything is valid, update the result and set the condition to true. + result.AuthorizationURL = *authURL + return &v1alpha1.Condition{ + Type: typeOIDCDiscoverySucceeded, + Status: v1alpha1.ConditionTrue, + Reason: reasonSuccess, + Message: "discovered issuer configuration", + } +} + +func (c *controller) updateStatus(ctx context.Context, upstream *v1alpha1.UpstreamOIDCProvider, conditions []*v1alpha1.Condition) { + log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) + updated := upstream.DeepCopy() + + updated.Status.Phase = v1alpha1.PhaseReady + + for i := range conditions { + cond := conditions[i].DeepCopy() + cond.LastTransitionTime = metav1.Now() + cond.ObservedGeneration = upstream.Generation + if mergeCondition(&updated.Status.Conditions, cond) { + log.Info("updated condition", "type", cond.Type, "status", cond.Status, "reason", cond.Reason, "message", cond.Message) + } + if cond.Status == v1alpha1.ConditionFalse { + updated.Status.Phase = v1alpha1.PhaseError + } + } + + sort.SliceStable(updated.Status.Conditions, func(i, j int) bool { + return updated.Status.Conditions[i].Type < updated.Status.Conditions[j].Type + }) + + if equality.Semantic.DeepEqual(upstream, updated) { + return + } + + _, err := c.client. + IDPV1alpha1(). + UpstreamOIDCProviders(upstream.Namespace). + UpdateStatus(ctx, updated, metav1.UpdateOptions{}) + if err != nil { + log.Error(err, "failed to update status") + } +} + +// mergeCondition merges a new v1alpha1.Condition into a slice of existing conditions. It returns true +// if the condition has meaningfully changed. +func mergeCondition(existing *[]v1alpha1.Condition, new *v1alpha1.Condition) bool { + // Find any existing condition with a matching type. + var old *v1alpha1.Condition + for i := range *existing { + if (*existing)[i].Type == new.Type { + old = &(*existing)[i] + continue + } + } + + // If there is no existing condition of this type, append this one and we're done. + if old == nil { + *existing = append(*existing, *new) + return true + } + + // Set the LastTransitionTime depending on whether the status has changed. + new = new.DeepCopy() + if old.Status == new.Status { + new.LastTransitionTime = old.LastTransitionTime + } + + // If anything has actually changed, update the entry and return true. + if !equality.Semantic.DeepEqual(old, new) { + *old = *new + return true + } + + // Otherwise the entry is already up to date. + return false +} + +func computeScopes(additionalScopes []string) []string { + // First compute the unique set of scopes, including "openid" (de-duplicate). + set := make(map[string]bool, len(additionalScopes)+1) + set["openid"] = true + for _, s := range additionalScopes { + set[s] = true + } + + // Then grab all the keys and sort them. + scopes := make([]string, 0, len(set)) + for s := range set { + scopes = append(scopes, s) + } + sort.Strings(scopes) + return scopes +} diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go new file mode 100644 index 00000000..5ac79bff --- /dev/null +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go @@ -0,0 +1,529 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package upstreamwatcher + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "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/client-go/informers" + "k8s.io/client-go/kubernetes/fake" + + "go.pinniped.dev/generated/1.19/apis/supervisor/idp/v1alpha1" + pinnipedfake "go.pinniped.dev/generated/1.19/client/supervisor/clientset/versioned/fake" + pinnipedinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/testutil/testlogger" +) + +func TestController(t *testing.T) { + t.Parallel() + now := metav1.NewTime(time.Now().UTC()) + earlier := metav1.NewTime(now.Add(-1 * time.Hour).UTC()) + + // Start another test server that answers discovery successfully. + testIssuer := newTestIssuer(t) + testIssuerAuthorizeURL, err := url.Parse("https://example.com/authorize") + require.NoError(t, err) + + var ( + testNamespace = "test-namespace" + testName = "test-name" + testSecretName = "test-client-secret" + testAdditionalScopes = []string{"scope1", "scope2", "scope3"} + testExpectedScopes = []string{"openid", "scope1", "scope2", "scope3"} + testClientID = "test-oidc-client-id" + testClientSecret = "test-oidc-client-secret" + testValidSecretData = map[string][]byte{"clientID": []byte(testClientID), "clientSecret": []byte(testClientSecret)} + ) + tests := []struct { + name string + inputUpstreams []runtime.Object + inputSecrets []runtime.Object + wantErr string + wantLogs []string + wantResultingCache []provider.UpstreamOIDCIdentityProvider + wantResultingUpstreams []v1alpha1.UpstreamOIDCProvider + }{ + { + name: "no upstreams", + }, + { + name: "missing secret", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuer.URL, + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, + }, + }}, + inputSecrets: []runtime.Object{}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="secret \"test-client-secret\" not found" "reason"="SecretNotFound" "status"="False" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, + `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="secret \"test-client-secret\" not found" "name"="test-name" "namespace"="test-namespace" "reason"="SecretNotFound" "type"="ClientCredentialsValid"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ClientCredentialsValid", + Status: "False", + LastTransitionTime: now, + Reason: "SecretNotFound", + Message: `secret "test-client-secret" not found`, + }, + { + Type: "OIDCDiscoverySucceeded", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "discovered issuer configuration", + }, + }, + }, + }}, + }, + { + name: "secret has wrong type", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuer.URL, + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "some-other-type", + Data: testValidSecretData, + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="referenced Secret \"test-client-secret\" has wrong type \"some-other-type\" (should be \"secrets.pinniped.dev/oidc-client\")" "reason"="SecretWrongType" "status"="False" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, + `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="referenced Secret \"test-client-secret\" has wrong type \"some-other-type\" (should be \"secrets.pinniped.dev/oidc-client\")" "name"="test-name" "namespace"="test-namespace" "reason"="SecretWrongType" "type"="ClientCredentialsValid"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ClientCredentialsValid", + Status: "False", + LastTransitionTime: now, + Reason: "SecretWrongType", + Message: `referenced Secret "test-client-secret" has wrong type "some-other-type" (should be "secrets.pinniped.dev/oidc-client")`, + }, + { + Type: "OIDCDiscoverySucceeded", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "discovered issuer configuration", + }, + }, + }, + }}, + }, + { + name: "secret is missing key", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuer.URL, + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "secrets.pinniped.dev/oidc-client", + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="referenced Secret \"test-client-secret\" is missing required keys [\"clientID\" \"clientSecret\"]" "reason"="SecretMissingKeys" "status"="False" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, + `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="referenced Secret \"test-client-secret\" is missing required keys [\"clientID\" \"clientSecret\"]" "name"="test-name" "namespace"="test-namespace" "reason"="SecretMissingKeys" "type"="ClientCredentialsValid"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ClientCredentialsValid", + Status: "False", + LastTransitionTime: now, + Reason: "SecretMissingKeys", + Message: `referenced Secret "test-client-secret" is missing required keys ["clientID" "clientSecret"]`, + }, + { + Type: "OIDCDiscoverySucceeded", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "discovered issuer configuration", + }, + }, + }, + }}, + }, + { + name: "issuer is invalid URL", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: "invalid-url", + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "secrets.pinniped.dev/oidc-client", + Data: testValidSecretData, + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to perform OIDC discovery against \"invalid-url\": Get \"invalid-url/.well-known/openid-configuration\": unsupported protocol scheme \"\"" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="failed to perform OIDC discovery against \"invalid-url\": Get \"invalid-url/.well-known/openid-configuration\": unsupported protocol scheme \"\"" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ClientCredentialsValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "loaded client credentials", + }, + { + Type: "OIDCDiscoverySucceeded", + Status: "False", + LastTransitionTime: now, + Reason: "Unreachable", + Message: `failed to perform OIDC discovery against "invalid-url": Get "invalid-url/.well-known/openid-configuration": unsupported protocol scheme ""`, + }, + }, + }, + }}, + }, + { + name: "issuer returns invalid authorize URL", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuer.URL + "/invalid", + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "secrets.pinniped.dev/oidc-client", + Data: testValidSecretData, + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to parse authorization endpoint URL: parse \"%\": invalid URL escape \"%\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="failed to parse authorization endpoint URL: parse \"%\": invalid URL escape \"%\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ClientCredentialsValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "loaded client credentials", + }, + { + Type: "OIDCDiscoverySucceeded", + Status: "False", + LastTransitionTime: now, + Reason: "InvalidResponse", + Message: `failed to parse authorization endpoint URL: parse "%": invalid URL escape "%"`, + }, + }, + }, + }}, + }, + { + name: "issuer returns insecure authorize URL", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuer.URL + "/insecure", + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "secrets.pinniped.dev/oidc-client", + Data: testValidSecretData, + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="authorization endpoint URL scheme must be \"https\", not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="authorization endpoint URL scheme must be \"https\", not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ClientCredentialsValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "loaded client credentials", + }, + { + Type: "OIDCDiscoverySucceeded", + Status: "False", + LastTransitionTime: now, + Reason: "InvalidResponse", + Message: `authorization endpoint URL scheme must be "https", not "http"`, + }, + }, + }, + }}, + }, + { + name: "upstream becomes valid", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name"}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuer.URL, + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: append(testAdditionalScopes, "xyz", "openid")}, + }, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + {Type: "ClientCredentialsValid", Status: "False", LastTransitionTime: earlier, Reason: "SomeError1", Message: "some previous error 1"}, + {Type: "OIDCDiscoverySucceeded", Status: "False", LastTransitionTime: earlier, Reason: "SomeError2", Message: "some previous error 2"}, + }, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "secrets.pinniped.dev/oidc-client", + Data: testValidSecretData, + }}, + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{{ + Name: testName, + ClientID: testClientID, + AuthorizationURL: *testIssuerAuthorizeURL, + Scopes: append(testExpectedScopes, "xyz"), + }}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Ready", + Conditions: []v1alpha1.Condition{ + {Type: "ClientCredentialsValid", Status: "True", LastTransitionTime: now, Reason: "Success", Message: "loaded client credentials"}, + {Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: now, Reason: "Success", Message: "discovered issuer configuration"}, + }, + }, + }}, + }, + { + name: "existing valid upstream", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuer.URL, + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, + }, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Ready", + Conditions: []v1alpha1.Condition{ + {Type: "ClientCredentialsValid", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "loaded client credentials"}, + {Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "discovered issuer configuration"}, + }, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "secrets.pinniped.dev/oidc-client", + Data: testValidSecretData, + }}, + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{{ + Name: testName, + ClientID: testClientID, + AuthorizationURL: *testIssuerAuthorizeURL, + Scopes: testExpectedScopes, + }}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Ready", + Conditions: []v1alpha1.Condition{ + {Type: "ClientCredentialsValid", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "loaded client credentials", ObservedGeneration: 1234}, + {Type: "OIDCDiscoverySucceeded", Status: "True", LastTransitionTime: earlier, Reason: "Success", Message: "discovered issuer configuration", ObservedGeneration: 1234}, + }, + }, + }}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + fakePinnipedClient := pinnipedfake.NewSimpleClientset(tt.inputUpstreams...) + pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClient, 0) + fakeKubeClient := fake.NewSimpleClientset(tt.inputSecrets...) + kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) + testLog := testlogger.New(t) + cache := provider.NewDynamicUpstreamIDPProvider() + cache.SetIDPList([]provider.UpstreamOIDCIdentityProvider{{Name: "initial-entry"}}) + + controller := New( + cache, + fakePinnipedClient, + pinnipedInformers.IDP().V1alpha1().UpstreamOIDCProviders(), + kubeInformers.Core().V1().Secrets(), + testLog) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + pinnipedInformers.Start(ctx.Done()) + kubeInformers.Start(ctx.Done()) + controllerlib.TestRunSynchronously(t, controller) + + syncCtx := controllerlib.Context{Context: ctx, Key: controllerlib.Key{}} + + if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, strings.Join(tt.wantLogs, "\n"), strings.Join(testLog.Lines(), "\n")) + require.ElementsMatch(t, tt.wantResultingCache, cache.GetIDPList()) + + actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().UpstreamOIDCProviders(testNamespace).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + // Preprocess the set of upstreams a bit so that they're easier to assert against. + require.ElementsMatch(t, tt.wantResultingUpstreams, normalizeUpstreams(actualUpstreams.Items, now)) + + // Running the sync() a second time should be idempotent except for logs, and should return the same error. + // This also helps exercise code paths where the OIDC provider discovery hits cache. + if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func normalizeUpstreams(upstreams []v1alpha1.UpstreamOIDCProvider, now metav1.Time) []v1alpha1.UpstreamOIDCProvider { + result := make([]v1alpha1.UpstreamOIDCProvider, 0, len(upstreams)) + for _, u := range upstreams { + normalized := u.DeepCopy() + + // We're only interested in comparing the status, so zero out the spec. + normalized.Spec = v1alpha1.UpstreamOIDCProviderSpec{} + + // Round down the LastTransitionTime values to `now` if they were just updated. This makes + // it much easier to encode assertions about the expected timestamps. + for i := range normalized.Status.Conditions { + if time.Since(normalized.Status.Conditions[i].LastTransitionTime.Time) < 5*time.Second { + normalized.Status.Conditions[i].LastTransitionTime = now + } + } + result = append(result, *normalized) + } + + return result +} + +func newTestIssuer(t *testing.T) *httptest.Server { + mux := http.NewServeMux() + testServer := httptest.NewServer(mux) + t.Cleanup(testServer.Close) + + type providerJSON struct { + Issuer string `json:"issuer"` + AuthURL string `json:"authorization_endpoint"` + TokenURL string `json:"token_endpoint"` + JWKSURL string `json:"jwks_uri"` + } + + // At the root of the server, serve an issuer with a valid discovery response. + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("content-type", "application/json") + _ = json.NewEncoder(w).Encode(&providerJSON{ + Issuer: testServer.URL, + AuthURL: "https://example.com/authorize", + }) + }) + + // At "/invalid", serve an issuer that returns an invalid authorization URL (not parseable). + mux.HandleFunc("/invalid/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("content-type", "application/json") + _ = json.NewEncoder(w).Encode(&providerJSON{ + Issuer: testServer.URL + "/invalid", + AuthURL: "%", + }) + }) + + // At "/insecure", serve an issuer that returns an insecure authorization URL (not https://). + mux.HandleFunc("/insecure/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("content-type", "application/json") + _ = json.NewEncoder(w).Encode(&providerJSON{ + Issuer: testServer.URL + "/insecure", + AuthURL: "http://example.com/authorize", + }) + }) + + return testServer +}