diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index 0cf897ae..3ac3ad1e 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -15,6 +15,8 @@ import ( "strings" "time" + "go.pinniped.dev/internal/controller/supervisorconfig/activedirectoryupstreamwatcher" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/clock" @@ -251,6 +253,15 @@ func startControllers( secretInformer, controllerlib.WithInformer, ), + singletonWorker). + WithController( + activedirectoryupstreamwatcher.New( + dynamicUpstreamIDPProvider, + pinnipedClient, + pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders(), + secretInformer, + controllerlib.WithInformer, + ), singletonWorker) kubeInformers.Start(ctx.Done()) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go new file mode 100644 index 00000000..5936c6bd --- /dev/null +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -0,0 +1,409 @@ +// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package activedirectoryupstreamwatcher implements a controller which watches LDAPIdentityProviders. +package activedirectoryupstreamwatcher + +import ( + "context" + "crypto/x509" + "encoding/base64" + "fmt" + "time" + + "go.pinniped.dev/internal/upstreamad" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/klog/v2/klogr" + + "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" + idpinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/idp/v1alpha1" + pinnipedcontroller "go.pinniped.dev/internal/controller" + "go.pinniped.dev/internal/controller/conditionsutil" + "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/upstreamldap" +) + +const ( + activeDirectoryControllerName = "active-directory-upstream-observer" + activeDirectoryBindAccountSecretType = corev1.SecretTypeBasicAuth + testActiveDirectoryConnectionTimeout = 90 * time.Second + + // Constants related to conditions. + typeBindSecretValid = "BindSecretValid" + typeTLSConfigurationValid = "TLSConfigurationValid" + typeActiveDirectoryConnectionValid = "ActiveDirectoryConnectionValid" + reasonActiveDirectoryConnectionError = "ActiveDirectoryConnectionError" + noTLSConfigurationMessage = "no TLS configuration provided" + loadedTLSConfigurationMessage = "loaded TLS configuration" +) + +// UpstreamActiveDirectoryIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. +type UpstreamActiveDirectoryIdentityProviderICache interface { + SetActiveDirectoryIdentityProviders([]provider.UpstreamLDAPIdentityProviderI) +} + +type activeDirectoryWatcherController struct { + cache UpstreamActiveDirectoryIdentityProviderICache + validatedSecretVersionsCache *secretVersionCache + ldapDialer upstreamldap.LDAPDialer + client pinnipedclientset.Interface + activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer + secretInformer corev1informers.SecretInformer +} + +// An in-memory cache with an entry for each ActiveDirectoryIdentityProvider, to keep track of which ResourceVersion +// of the bind Secret and which TLS/StartTLS setting was used during the most recent successful validation. +type secretVersionCache struct { + ValidatedSettingsByName map[string]validatedSettings +} + +type validatedSettings struct { + BindSecretResourceVersion string + LDAPConnectionProtocol upstreamldap.LDAPConnectionProtocol +} + +func newSecretVersionCache() *secretVersionCache { + return &secretVersionCache{ValidatedSettingsByName: map[string]validatedSettings{}} +} + +// New instantiates a new controllerlib.Controller which will populate the provided UpstreamActiveDirectoryIdentityProviderICache. +func New( + idpCache UpstreamActiveDirectoryIdentityProviderICache, + client pinnipedclientset.Interface, + activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer, + secretInformer corev1informers.SecretInformer, + withInformer pinnipedcontroller.WithInformerOptionFunc, +) controllerlib.Controller { + return newInternal( + idpCache, + // start with an empty secretVersionCache + newSecretVersionCache(), + // nil means to use a real production dialer when creating objects to add to the cache + nil, + client, + activeDirectoryIdentityProviderInformer, + secretInformer, + withInformer, + ) +} + +// For test dependency injection purposes. +func newInternal( + idpCache UpstreamActiveDirectoryIdentityProviderICache, + validatedSecretVersionsCache *secretVersionCache, + ldapDialer upstreamldap.LDAPDialer, + client pinnipedclientset.Interface, + activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer, + secretInformer corev1informers.SecretInformer, + withInformer pinnipedcontroller.WithInformerOptionFunc, +) controllerlib.Controller { + c := activeDirectoryWatcherController{ + cache: idpCache, + validatedSecretVersionsCache: validatedSecretVersionsCache, + ldapDialer: ldapDialer, + client: client, + activeDirectoryIdentityProviderInformer: activeDirectoryIdentityProviderInformer, + secretInformer: secretInformer, + } + return controllerlib.New( + controllerlib.Config{Name: activeDirectoryControllerName, Syncer: &c}, + withInformer( + activeDirectoryIdentityProviderInformer, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + withInformer( + secretInformer, + pinnipedcontroller.MatchAnySecretOfTypeFilter(activeDirectoryBindAccountSecretType, pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + ) +} + +// Sync implements controllerlib.Syncer. +func (c *activeDirectoryWatcherController) Sync(ctx controllerlib.Context) error { + actualUpstreams, err := c.activeDirectoryIdentityProviderInformer.Lister().List(labels.Everything()) + if err != nil { + return fmt.Errorf("failed to list LDAPIdentityProviders: %w", err) + } + + requeue := false + validatedUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, 0, len(actualUpstreams)) + for _, upstream := range actualUpstreams { + valid, requestedRequeue := c.validateUpstream(ctx.Context, upstream) + if valid != nil { + validatedUpstreams = append(validatedUpstreams, valid) + } + if requestedRequeue { + requeue = true + } + } + + c.cache.SetActiveDirectoryIdentityProviders(validatedUpstreams) + + if requeue { + return controllerlib.ErrSyntheticRequeue + } + return nil +} + +func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider) (p provider.UpstreamLDAPIdentityProviderI, requeue bool) { + spec := upstream.Spec + + config := &upstreamldap.ProviderConfig{ + Name: upstream.Name, + Host: spec.Host, + UserSearch: upstreamldap.UserSearchConfig{ + Base: spec.UserSearch.Base, + Filter: spec.UserSearch.Filter, + UsernameAttribute: spec.UserSearch.Attributes.Username, + UIDAttribute: spec.UserSearch.Attributes.UID, + }, + GroupSearch: upstreamldap.GroupSearchConfig{ + Base: spec.GroupSearch.Base, + Filter: spec.GroupSearch.Filter, + GroupNameAttribute: spec.GroupSearch.Attributes.GroupName, + }, + Dialer: c.ldapDialer, + } + + conditions := []*v1alpha1.Condition{} + secretValidCondition, currentSecretVersion := c.validateSecret(upstream, config) + tlsValidCondition := c.validateTLSConfig(upstream, config) + conditions = append(conditions, secretValidCondition, tlsValidCondition) + + // No point in trying to connect to the server if the config was already determined to be invalid. + var finishedConfigCondition *v1alpha1.Condition + if secretValidCondition.Status == v1alpha1.ConditionTrue && tlsValidCondition.Status == v1alpha1.ConditionTrue { + finishedConfigCondition = c.validateFinishedConfig(ctx, upstream, config, currentSecretVersion) + if finishedConfigCondition != nil { + conditions = append(conditions, finishedConfigCondition) + } + } + + c.updateStatus(ctx, upstream, conditions) + + switch { + case secretValidCondition.Status != v1alpha1.ConditionTrue || tlsValidCondition.Status != v1alpha1.ConditionTrue: + // Invalid provider, so do not load it into the cache. + p = nil + requeue = true + case finishedConfigCondition != nil && finishedConfigCondition.Status != v1alpha1.ConditionTrue: + // Error but load it into the cache anyway, treating this condition failure more like a warning. + p = upstreamad.New(*config) + // Try again hoping that the condition will improve. + requeue = true + default: + // Fully validated provider, so load it into the cache. + p = upstreamad.New(*config) + requeue = false + } + + return p, requeue +} + +func (c *activeDirectoryWatcherController) validateTLSConfig(upstream *v1alpha1.ActiveDirectoryIdentityProvider, config *upstreamldap.ProviderConfig) *v1alpha1.Condition { + tlsSpec := upstream.Spec.TLS + if tlsSpec == nil { + return c.validTLSCondition(noTLSConfigurationMessage) + } + if len(tlsSpec.CertificateAuthorityData) == 0 { + return c.validTLSCondition(loadedTLSConfigurationMessage) + } + + bundle, err := base64.StdEncoding.DecodeString(tlsSpec.CertificateAuthorityData) + if err != nil { + return c.invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", err.Error())) + } + + ca := x509.NewCertPool() + ok := ca.AppendCertsFromPEM(bundle) + if !ok { + return c.invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", upstreamwatchers.ErrNoCertificates)) + } + + config.CABundle = bundle + return c.validTLSCondition(loadedTLSConfigurationMessage) +} + +func (c *activeDirectoryWatcherController) validateFinishedConfig(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider, config *upstreamldap.ProviderConfig, currentSecretVersion string) *v1alpha1.Condition { + if c.hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream, currentSecretVersion, config) { + return nil + } + + testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, testActiveDirectoryConnectionTimeout) + defer cancelFunc() + + condition := c.testConnection(testConnectionTimeout, upstream, config, currentSecretVersion) + + if condition.Status == v1alpha1.ConditionTrue { + // Remember (in-memory for this pod) that the controller has successfully validated the LDAP provider + // using this version of the Secret. This is for performance reasons, to avoid attempting to connect to + // the LDAP server more than is needed. If the pod restarts, it will attempt this validation again. + c.validatedSecretVersionsCache.ValidatedSettingsByName[upstream.GetName()] = validatedSettings{ + BindSecretResourceVersion: currentSecretVersion, + LDAPConnectionProtocol: config.ConnectionProtocol, + } + } + + return condition +} + +func (c *activeDirectoryWatcherController) testConnection( + ctx context.Context, + upstream *v1alpha1.ActiveDirectoryIdentityProvider, + config *upstreamldap.ProviderConfig, + currentSecretVersion string, +) *v1alpha1.Condition { + // First try using TLS. + config.ConnectionProtocol = upstreamldap.TLS + tlsLDAPProvider := upstreamldap.New(*config) + err := tlsLDAPProvider.TestConnection(ctx) + if err != nil { + plog.InfoErr("testing LDAP connection using TLS failed, so trying again with StartTLS", err, "host", config.Host) + // If there was any error, try again with StartTLS instead. + config.ConnectionProtocol = upstreamldap.StartTLS + startTLSLDAPProvider := upstreamldap.New(*config) + startTLSErr := startTLSLDAPProvider.TestConnection(ctx) + if startTLSErr == nil { + plog.Info("testing LDAP connection using StartTLS succeeded", "host", config.Host) + // Successfully able to fall back to using StartTLS, so clear the original + // error and consider the connection test to be successful. + err = nil + } else { + plog.InfoErr("testing LDAP connection using StartTLS also failed", err, "host", config.Host) + // Falling back to StartTLS also failed, so put TLS back into the config + // and consider the connection test to be failed. + config.ConnectionProtocol = upstreamldap.TLS + } + } + + if err != nil { + return &v1alpha1.Condition{ + Type: typeActiveDirectoryConnectionValid, + Status: v1alpha1.ConditionFalse, + Reason: reasonActiveDirectoryConnectionError, + Message: fmt.Sprintf(`could not successfully connect to "%s" and bind as user "%s": %s`, + config.Host, config.BindUsername, err.Error()), + } + } + + return &v1alpha1.Condition{ + Type: typeActiveDirectoryConnectionValid, + Status: v1alpha1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf(`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + config.Host, config.BindUsername, upstream.Spec.Bind.SecretName, currentSecretVersion), + } +} + +func (c *activeDirectoryWatcherController) hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream *v1alpha1.ActiveDirectoryIdentityProvider, currentSecretVersion string, config *upstreamldap.ProviderConfig) bool { + currentGeneration := upstream.Generation + for _, cond := range upstream.Status.Conditions { + if cond.Type == typeActiveDirectoryConnectionValid && cond.Status == v1alpha1.ConditionTrue && cond.ObservedGeneration == currentGeneration { + // Found a previously successful condition for the current spec generation. + // Now figure out which version of the bind Secret was used during that previous validation, if any. + validatedSecretVersion := c.validatedSecretVersionsCache.ValidatedSettingsByName[upstream.GetName()] + if validatedSecretVersion.BindSecretResourceVersion == currentSecretVersion { + // Reload the TLS vs StartTLS setting that was previously validated. + config.ConnectionProtocol = validatedSecretVersion.LDAPConnectionProtocol + return true + } + } + } + return false +} + +func (c *activeDirectoryWatcherController) validTLSCondition(message string) *v1alpha1.Condition { + return &v1alpha1.Condition{ + Type: typeTLSConfigurationValid, + Status: v1alpha1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: message, + } +} + +func (c *activeDirectoryWatcherController) invalidTLSCondition(message string) *v1alpha1.Condition { + return &v1alpha1.Condition{ + Type: typeTLSConfigurationValid, + Status: v1alpha1.ConditionFalse, + Reason: upstreamwatchers.ReasonInvalidTLSConfig, + Message: message, + } +} + +func (c *activeDirectoryWatcherController) validateSecret(upstream *v1alpha1.ActiveDirectoryIdentityProvider, config *upstreamldap.ProviderConfig) (*v1alpha1.Condition, string) { + secretName := upstream.Spec.Bind.SecretName + + secret, err := c.secretInformer.Lister().Secrets(upstream.Namespace).Get(secretName) + if err != nil { + return &v1alpha1.Condition{ + Type: typeBindSecretValid, + Status: v1alpha1.ConditionFalse, + Reason: upstreamwatchers.ReasonNotFound, + Message: err.Error(), + }, "" + } + + if secret.Type != corev1.SecretTypeBasicAuth { + return &v1alpha1.Condition{ + Type: typeBindSecretValid, + Status: v1alpha1.ConditionFalse, + Reason: upstreamwatchers.ReasonWrongType, + Message: fmt.Sprintf("referenced Secret %q has wrong type %q (should be %q)", + secretName, secret.Type, corev1.SecretTypeBasicAuth), + }, secret.ResourceVersion + } + + config.BindUsername = string(secret.Data[corev1.BasicAuthUsernameKey]) + config.BindPassword = string(secret.Data[corev1.BasicAuthPasswordKey]) + if len(config.BindUsername) == 0 || len(config.BindPassword) == 0 { + return &v1alpha1.Condition{ + Type: typeBindSecretValid, + Status: v1alpha1.ConditionFalse, + Reason: upstreamwatchers.ReasonMissingKeys, + Message: fmt.Sprintf("referenced Secret %q is missing required keys %q", + secretName, []string{corev1.BasicAuthUsernameKey, corev1.BasicAuthPasswordKey}), + }, secret.ResourceVersion + } + + return &v1alpha1.Condition{ + Type: typeBindSecretValid, + Status: v1alpha1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: "loaded bind secret", + }, secret.ResourceVersion +} + +func (c *activeDirectoryWatcherController) updateStatus(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider, conditions []*v1alpha1.Condition) { + log := klogr.New().WithValues("namespace", upstream.Namespace, "name", upstream.Name) + updated := upstream.DeepCopy() + + hadErrorCondition := conditionsutil.Merge(conditions, upstream.Generation, &updated.Status.Conditions, log) + + updated.Status.Phase = v1alpha1.ActiveDirectoryPhaseReady + if hadErrorCondition { + updated.Status.Phase = v1alpha1.ActiveDirectoryPhaseError + } + + if equality.Semantic.DeepEqual(upstream, updated) { + return // nothing to update + } + + _, err := c.client. + IDPV1alpha1(). + ActiveDirectoryIdentityProviders(upstream.Namespace). + UpdateStatus(ctx, updated, metav1.UpdateOptions{}) + if err != nil { + log.Error(err, "failed to update status") + } +} diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go new file mode 100644 index 00000000..1b7ae2aa --- /dev/null +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -0,0 +1,971 @@ +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package activedirectoryupstreamwatcher + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "sort" + "testing" + "time" + + "go.pinniped.dev/internal/upstreamad" + + "github.com/go-ldap/ldap/v3" + "github.com/golang/mock/gomock" + "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/latest/apis/supervisor/idp/v1alpha1" + pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake" + pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" + "go.pinniped.dev/internal/certauthority" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/endpointaddr" + "go.pinniped.dev/internal/mocks/mockldapconn" + "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/internal/upstreamldap" +) + +func TestLDAPUpstreamWatcherControllerFilterSecrets(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + secret metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "a secret of the right type", + secret: &corev1.Secret{ + Type: corev1.SecretTypeBasicAuth, + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "a secret of the wrong type", + secret: &corev1.Secret{ + Type: "this-is-the-wrong-type", + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + }, + { + name: "resource of a data type which is not watched by this controller", + secret: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + fakePinnipedClient := pinnipedfake.NewSimpleClientset() + pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClient, 0) + activeDirectoryIDPInformer := pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders() + fakeKubeClient := fake.NewSimpleClientset() + kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) + secretInformer := kubeInformers.Core().V1().Secrets() + withInformer := testutil.NewObservableWithInformerOption() + + New(nil, nil, activeDirectoryIDPInformer, secretInformer, 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)) + }) + } +} + +func TestLDAPUpstreamWatcherControllerFilterLDAPIdentityProviders(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + idp metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "any LDAPIdentityProvider", + idp: &v1alpha1.ActiveDirectoryIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + fakePinnipedClient := pinnipedfake.NewSimpleClientset() + pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClient, 0) + activeDirectoryIDPInformer := pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders() + fakeKubeClient := fake.NewSimpleClientset() + kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) + secretInformer := kubeInformers.Core().V1().Secrets() + withInformer := testutil.NewObservableWithInformerOption() + + New(nil, nil, activeDirectoryIDPInformer, secretInformer, withInformer.WithInformer) + + unrelated := corev1.Secret{} + filter := withInformer.GetFilterForInformer(activeDirectoryIDPInformer) + require.Equal(t, test.wantAdd, filter.Add(test.idp)) + require.Equal(t, test.wantUpdate, filter.Update(&unrelated, test.idp)) + require.Equal(t, test.wantUpdate, filter.Update(test.idp, &unrelated)) + require.Equal(t, test.wantDelete, filter.Delete(test.idp)) + }) + } +} + +// Wrap the func into a struct so the test can do deep equal assertions on instances of upstreamad.Provider. +type comparableDialer struct { + upstreamldap.LDAPDialerFunc +} + +func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { + t.Parallel() + now := metav1.NewTime(time.Now().UTC()) + + const ( + testNamespace = "test-namespace" + testName = "test-name" + testSecretName = "test-bind-secret" + testBindUsername = "test-bind-username" + testBindPassword = "test-bind-password" + testHost = "ldap.example.com:123" + testUserSearchBase = "test-user-search-base" + testUserSearchFilter = "test-user-search-filter" + testGroupSearchBase = "test-group-search-base" + testGroupSearchFilter = "test-group-search-filter" + testUsernameAttrName = "test-username-attr" + testGroupNameAttrName = "test-group-name-attr" + testUIDAttrName = "test-uid-attr" + ) + + testValidSecretData := map[string][]byte{"username": []byte(testBindUsername), "password": []byte(testBindPassword)} + + testCA, err := certauthority.New("test CA", time.Minute) + require.NoError(t, err) + testCABundle := testCA.Bundle() + testCABundleBase64Encoded := base64.StdEncoding.EncodeToString(testCABundle) + + validUpstream := &v1alpha1.ActiveDirectoryIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{Name: testName, Namespace: testNamespace, Generation: 1234}, + Spec: v1alpha1.ActiveDirectoryIdentityProviderSpec{ + Host: testHost, + TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testCABundleBase64Encoded}, + Bind: v1alpha1.ActiveDirectoryIdentityProviderBind{SecretName: testSecretName}, + UserSearch: v1alpha1.ActiveDirectoryIdentityProviderUserSearch{ + Base: testUserSearchBase, + Filter: testUserSearchFilter, + Attributes: v1alpha1.ActiveDirectoryIdentityProviderUserSearchAttributes{ + Username: testUsernameAttrName, + UID: testUIDAttrName, + }, + }, + GroupSearch: v1alpha1.ActiveDirectoryIdentityProviderGroupSearch{ + Base: testGroupSearchBase, + Filter: testGroupSearchFilter, + Attributes: v1alpha1.ActiveDirectoryIdentityProviderGroupSearchAttributes{ + GroupName: testGroupNameAttrName, + }, + }, + }, + } + editedValidUpstream := func(editFunc func(*v1alpha1.ActiveDirectoryIdentityProvider)) *v1alpha1.ActiveDirectoryIdentityProvider { + deepCopy := validUpstream.DeepCopy() + editFunc(deepCopy) + return deepCopy + } + + providerConfigForValidUpstreamWithTLS := &upstreamldap.ProviderConfig{ + Name: testName, + Host: testHost, + ConnectionProtocol: upstreamldap.TLS, + CABundle: testCABundle, + BindUsername: testBindUsername, + BindPassword: testBindPassword, + UserSearch: upstreamldap.UserSearchConfig{ + Base: testUserSearchBase, + Filter: testUserSearchFilter, + UsernameAttribute: testUsernameAttrName, + UIDAttribute: testUIDAttrName, + }, + GroupSearch: upstreamldap.GroupSearchConfig{ + Base: testGroupSearchBase, + Filter: testGroupSearchFilter, + GroupNameAttribute: testGroupNameAttrName, + }, + } + + // Make a copy with targeted changes. + copyOfProviderConfigForValidUpstreamWithTLS := *providerConfigForValidUpstreamWithTLS + providerConfigForValidUpstreamWithStartTLS := ©OfProviderConfigForValidUpstreamWithTLS + providerConfigForValidUpstreamWithStartTLS.ConnectionProtocol = upstreamldap.StartTLS + + bindSecretValidTrueCondition := func(gen int64) v1alpha1.Condition { + return v1alpha1.Condition{ + Type: "BindSecretValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "loaded bind secret", + ObservedGeneration: gen, + } + } + activeDirectoryConnectionValidTrueCondition := func(gen int64, secretVersion string) v1alpha1.Condition { + return v1alpha1.Condition{ + Type: "ActiveDirectoryConnectionValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + testHost, testBindUsername, testSecretName, secretVersion), + ObservedGeneration: gen, + } + } + tlsConfigurationValidLoadedTrueCondition := func(gen int64) v1alpha1.Condition { + return v1alpha1.Condition{ + Type: "TLSConfigurationValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "loaded TLS configuration", + ObservedGeneration: gen, + } + } + allConditionsTrue := func(gen int64, secretVersion string) []v1alpha1.Condition { + return []v1alpha1.Condition{ + activeDirectoryConnectionValidTrueCondition(gen, secretVersion), + bindSecretValidTrueCondition(gen), + tlsConfigurationValidLoadedTrueCondition(gen), + } + } + + validBindUserSecret := func(secretVersion string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace, ResourceVersion: secretVersion}, + Type: corev1.SecretTypeBasicAuth, + Data: testValidSecretData, + } + } + + tests := []struct { + name string + initialValidatedSettings map[string]validatedSettings + inputUpstreams []runtime.Object + inputSecrets []runtime.Object + setupMocks func(conn *mockldapconn.MockConn) + dialErrors map[string]error + wantErr string + wantResultingCache []*upstreamldap.ProviderConfig + wantResultingUpstreams []v1alpha1.ActiveDirectoryIdentityProvider + wantValidatedSettings map[string]validatedSettings + }{ + { + name: "no ActiveDirectoryIdentityProvider upstreams clears the cache", + wantResultingCache: []*upstreamldap.ProviderConfig{}, + }, + { + name: "one valid upstream updates the cache to include only that upstream", + inputUpstreams: []runtime.Object{validUpstream}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + }, + { + name: "missing secret", + inputUpstreams: []runtime.Object{validUpstream}, + inputSecrets: []runtime.Object{}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "BindSecretValid", + Status: "False", + LastTransitionTime: now, + Reason: "SecretNotFound", + Message: fmt.Sprintf(`secret "%s" not found`, testSecretName), + ObservedGeneration: 1234, + }, + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + }, + { + name: "secret has wrong type", + inputUpstreams: []runtime.Object{validUpstream}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace}, + Type: "some-other-type", + Data: testValidSecretData, + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "BindSecretValid", + Status: "False", + LastTransitionTime: now, + Reason: "SecretWrongType", + Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testSecretName), + ObservedGeneration: 1234, + }, + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + }, + { + name: "secret is missing key", + inputUpstreams: []runtime.Object{validUpstream}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace}, + Type: corev1.SecretTypeBasicAuth, + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "BindSecretValid", + Status: "False", + LastTransitionTime: now, + Reason: "SecretMissingKeys", + Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testSecretName), + ObservedGeneration: 1234, + }, + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + }, + { + name: "CertificateAuthorityData is not base64 encoded", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Spec.TLS.CertificateAuthorityData = "this-is-not-base64-encoded" + })}, + inputSecrets: []runtime.Object{validBindUserSecret("")}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), + { + Type: "TLSConfigurationValid", + Status: "False", + LastTransitionTime: now, + Reason: "InvalidTLSConfig", + Message: "certificateAuthorityData is invalid: illegal base64 data at input byte 4", + ObservedGeneration: 1234, + }, + }, + }, + }}, + }, + { + name: "CertificateAuthorityData is not valid pem data", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Spec.TLS.CertificateAuthorityData = base64.StdEncoding.EncodeToString([]byte("this is not pem data")) + })}, + inputSecrets: []runtime.Object{validBindUserSecret("")}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), + { + Type: "TLSConfigurationValid", + Status: "False", + LastTransitionTime: now, + Reason: "InvalidTLSConfig", + Message: "certificateAuthorityData is invalid: no certificates found", + ObservedGeneration: 1234, + }, + }, + }, + }}, + }, + { + name: "nil TLS configuration is valid", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Spec.TLS = nil + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{ + { + Name: testName, + Host: testHost, + ConnectionProtocol: upstreamldap.TLS, + CABundle: nil, + BindUsername: testBindUsername, + BindPassword: testBindPassword, + UserSearch: upstreamldap.UserSearchConfig{ + Base: testUserSearchBase, + Filter: testUserSearchFilter, + UsernameAttribute: testUsernameAttrName, + UIDAttribute: testUIDAttrName, + }, + GroupSearch: upstreamldap.GroupSearchConfig{ + Base: testGroupSearchBase, + Filter: testGroupSearchFilter, + GroupNameAttribute: testGroupNameAttrName, + }, + }, + }, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: []v1alpha1.Condition{ + activeDirectoryConnectionValidTrueCondition(1234, "4242"), + bindSecretValidTrueCondition(1234), + { + Type: "TLSConfigurationValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "no TLS configuration provided", + ObservedGeneration: 1234, + }, + }, + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + }, + { + name: "when TLS connection fails it tries to use StartTLS instead: without a specified port it automatically switches ports", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Spec.Host = "ldap.example.com" // when the port is not specified, automatically switch ports for StartTLS + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Close().Times(1) + }, + dialErrors: map[string]error{ + "ldap.example.com:" + ldap.DefaultLdapsPort: fmt.Errorf("some ldaps dial error"), + "ldap.example.com:" + ldap.DefaultLdapPort: nil, // no error on the regular ldap:// port + }, + wantResultingCache: []*upstreamldap.ProviderConfig{ + { + Name: testName, + Host: "ldap.example.com", + ConnectionProtocol: upstreamldap.StartTLS, // successfully fell back to using StartTLS + CABundle: testCABundle, + BindUsername: testBindUsername, + BindPassword: testBindPassword, + UserSearch: upstreamldap.UserSearchConfig{ + Base: testUserSearchBase, + Filter: testUserSearchFilter, + UsernameAttribute: testUsernameAttrName, + UIDAttribute: testUIDAttrName, + }, + GroupSearch: upstreamldap.GroupSearchConfig{ + Base: testGroupSearchBase, + Filter: testGroupSearchFilter, + GroupNameAttribute: testGroupNameAttrName, + }, + }, + }, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: []v1alpha1.Condition{ + { + Type: "ActiveDirectoryConnectionValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + "ldap.example.com", testBindUsername, testSecretName, "4242"), + ObservedGeneration: 1234, + }, + bindSecretValidTrueCondition(1234), + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + }, + { + name: "when TLS connection fails it tries to use StartTLS instead: with a specified port it does not automatically switch ports", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Spec.Host = "ldap.example.com:5678" // when the port is specified, do not automatically switch ports for StartTLS + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Both dials fail, so there should be no bind. + }, + dialErrors: map[string]error{ + "ldap.example.com:5678": fmt.Errorf("some dial error"), // both TLS and StartTLS should try the same port and both fail + }, + wantResultingCache: []*upstreamldap.ProviderConfig{ + // even though the connection test failed, still loads into the cache because it is treated like a warning + { + Name: testName, + Host: "ldap.example.com:5678", + ConnectionProtocol: upstreamldap.TLS, // need to pick TLS or StartTLS to load into the cache when both fail, so choose TLS + CABundle: testCABundle, + BindUsername: testBindUsername, + BindPassword: testBindPassword, + UserSearch: upstreamldap.UserSearchConfig{ + Base: testUserSearchBase, + Filter: testUserSearchFilter, + UsernameAttribute: testUsernameAttrName, + UIDAttribute: testUIDAttrName, + }, + GroupSearch: upstreamldap.GroupSearchConfig{ + Base: testGroupSearchBase, + Filter: testGroupSearchFilter, + GroupNameAttribute: testGroupNameAttrName, + }, + }, + }, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ActiveDirectoryConnectionValid", + Status: "False", + LastTransitionTime: now, + Reason: "ActiveDirectoryConnectionError", + Message: fmt.Sprintf( + `could not successfully connect to "%s" and bind as user "%s": error dialing host "%s": some dial error`, + "ldap.example.com:5678", testBindUsername, "ldap.example.com:5678"), + ObservedGeneration: 1234, + }, + bindSecretValidTrueCondition(1234), + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + }, + { + name: "non-nil TLS configuration with empty CertificateAuthorityData is valid", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Spec.TLS.CertificateAuthorityData = "" + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{ + { + Name: testName, + Host: testHost, + ConnectionProtocol: upstreamldap.TLS, + CABundle: nil, + BindUsername: testBindUsername, + BindPassword: testBindPassword, + UserSearch: upstreamldap.UserSearchConfig{ + Base: testUserSearchBase, + Filter: testUserSearchFilter, + UsernameAttribute: testUsernameAttrName, + UIDAttribute: testUIDAttrName, + }, + GroupSearch: upstreamldap.GroupSearchConfig{ + Base: testGroupSearchBase, + Filter: testGroupSearchFilter, + GroupNameAttribute: testGroupNameAttrName, + }, + }, + }, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + }, + { + name: "one valid upstream and one invalid upstream updates the cache to include only the valid upstream", + inputUpstreams: []runtime.Object{validUpstream, editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Name = "other-upstream" + upstream.Generation = 42 + upstream.Spec.Bind.SecretName = "non-existent-secret" + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind for the one valid upstream configuration. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{ + { + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-upstream", Generation: 42}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "BindSecretValid", + Status: "False", + LastTransitionTime: now, + Reason: "SecretNotFound", + Message: fmt.Sprintf(`secret "%s" not found`, "non-existent-secret"), + ObservedGeneration: 42, + }, + tlsConfigurationValidLoadedTrueCondition(42), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }, + }, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + }, + { + name: "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway (treated like a warning)", + inputUpstreams: []runtime.Object{validUpstream}, + inputSecrets: []runtime.Object{validBindUserSecret("")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + // Expect two calls to each of these: once for trying TLS and once for trying StartTLS. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(2).Return(errors.New("some bind error")) + conn.EXPECT().Close().Times(2) + }, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ActiveDirectoryConnectionValid", + Status: "False", + LastTransitionTime: now, + Reason: "ActiveDirectoryConnectionError", + Message: fmt.Sprintf( + `could not successfully connect to "%s" and bind as user "%s": error binding as "%s": some bind error`, + testHost, testBindUsername, testBindUsername), + ObservedGeneration: 1234, + }, + bindSecretValidTrueCondition(1234), + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + }, + { + name: "when the LDAP server connection was already validated using TLS for the current resource generation and secret version, then do not validate it again and keep using TLS", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Generation = 1234 + upstream.Status.Conditions = []v1alpha1.Condition{ + activeDirectoryConnectionValidTrueCondition(1234, "4242"), + } + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called. + }, + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + }, + { + name: "when the LDAP server connection was already validated using StartTLS for the current resource generation and secret version, then do not validate it again and keep using StartTLS", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Generation = 1234 + upstream.Status.Conditions = []v1alpha1.Condition{ + activeDirectoryConnectionValidTrueCondition(1234, "4242"), + } + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called. + }, + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithStartTLS}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + }, + { + name: "when the LDAP server connection was validated for an older resource generation, then try to validate it again", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Generation = 1234 // current generation + upstream.Status.Conditions = []v1alpha1.Condition{ + activeDirectoryConnectionValidTrueCondition(1233, "4242"), // older spec generation! + } + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + }, + { + name: "when the LDAP server connection validation previously failed for this resource generation, then try to validate it again", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Generation = 1234 + upstream.Status.Conditions = []v1alpha1.Condition{ + { + Type: "ActiveDirectoryConnectionValid", + Status: "False", // failure! + LastTransitionTime: now, + Reason: "ActiveDirectoryConnectionError", + Message: "some-error-message", + ObservedGeneration: 1234, // same (current) generation! + }, + } + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + }, + { + name: "when the LDAP server connection was already validated for this resource generation but the bind secret has changed, then try to validate it again", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Generation = 1234 + upstream.Status.Conditions = []v1alpha1.Condition{ + activeDirectoryConnectionValidTrueCondition(1234, "4241"), // same spec generation, old secret version + } + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, // newer secret version! + initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4241", LDAPConnectionProtocol: upstreamldap.TLS}}, // old version was validated + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + }, + } + + 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) + cache := provider.NewDynamicUpstreamIDPProvider() + cache.SetActiveDirectoryIdentityProviders([]provider.UpstreamLDAPIdentityProviderI{ + upstreamad.New(upstreamldap.ProviderConfig{Name: "initial-entry"}), + }) + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + conn := mockldapconn.NewMockConn(ctrl) + if tt.setupMocks != nil { + tt.setupMocks(conn) + } + + dialer := &comparableDialer{upstreamldap.LDAPDialerFunc(func(ctx context.Context, addr endpointaddr.HostPort) (upstreamldap.Conn, error) { + if tt.dialErrors != nil { + dialErr := tt.dialErrors[addr.Endpoint()] + if dialErr != nil { + return nil, dialErr + } + } + return conn, nil + })} + + validatedSecretVersionCache := newSecretVersionCache() + if tt.initialValidatedSettings != nil { + validatedSecretVersionCache.ValidatedSettingsByName = tt.initialValidatedSettings + } + + controller := newInternal( + cache, + validatedSecretVersionCache, + dialer, + fakePinnipedClient, + pinnipedInformers.IDP().V1alpha1().ActiveDirectoryIdentityProviders(), + kubeInformers.Core().V1().Secrets(), + controllerlib.WithInformer, + ) + + ctx, cancel := context.WithCancel(context.Background()) + 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) + } + + actualIDPList := cache.GetActiveDirectoryIdentityProviders() + require.Equal(t, len(tt.wantResultingCache), len(actualIDPList)) + for i := range actualIDPList { + actualIDP := actualIDPList[i].(*upstreamad.Provider) + copyOfExpectedValueForResultingCache := *tt.wantResultingCache[i] // copy before edit to avoid race because these tests are run in parallel + // The dialer that was passed in to the controller's constructor should always have been + // passed through to the provider. + copyOfExpectedValueForResultingCache.Dialer = dialer + require.Equal(t, copyOfExpectedValueForResultingCache, actualIDP.GetConfig()) + } + + actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().ActiveDirectoryIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + // Assert on the expected Status of the upstreams. Preprocess the upstreams a bit so that they're easier to assert against. + normalizedActualUpstreams := normalizeActiveDirectoryUpstreams(actualUpstreams.Items, now) + require.Equal(t, len(tt.wantResultingUpstreams), len(normalizedActualUpstreams)) + for i := range tt.wantResultingUpstreams { + // Require each separately to get a nice diff when the test fails. + require.Equal(t, tt.wantResultingUpstreams[i], normalizedActualUpstreams[i]) + } + + // Check that the controller remembered which version of the secret it most recently validated successfully with. + if tt.wantValidatedSettings == nil { + tt.wantValidatedSettings = map[string]validatedSettings{} + } + require.Equal(t, tt.wantValidatedSettings, validatedSecretVersionCache.ValidatedSettingsByName) + }) + } +} + +func normalizeActiveDirectoryUpstreams(upstreams []v1alpha1.ActiveDirectoryIdentityProvider, now metav1.Time) []v1alpha1.ActiveDirectoryIdentityProvider { + result := make([]v1alpha1.ActiveDirectoryIdentityProvider, 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.ActiveDirectoryIdentityProviderSpec{} + + // 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) + } + + sort.SliceStable(result, func(i, j int) bool { + return result[i].Name < result[j].Name + }) + + return result +} diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 0f21ad05..87ebe876 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -260,17 +260,18 @@ func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken { return csrfFromCookie } -// Select either an OIDC or an LDAP IDP, or return an error. +// Select either an OIDC, an LDAP or an AD IDP, or return an error. func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider.UpstreamOIDCIdentityProviderI, provider.UpstreamLDAPIdentityProviderI, error) { oidcUpstreams := idpLister.GetOIDCIdentityProviders() ldapUpstreams := idpLister.GetLDAPIdentityProviders() + adUpstreams := idpLister.GetActiveDirectoryIdentityProviders() switch { - case len(oidcUpstreams)+len(ldapUpstreams) == 0: + case len(oidcUpstreams)+len(ldapUpstreams)+len(adUpstreams) == 0: return nil, nil, httperr.New( http.StatusUnprocessableEntity, "No upstream providers are configured", ) - case len(oidcUpstreams)+len(ldapUpstreams) > 1: + case len(oidcUpstreams)+len(ldapUpstreams)+len(adUpstreams) > 1: var upstreamIDPNames []string for _, idp := range oidcUpstreams { upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) @@ -278,6 +279,9 @@ func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider for _, idp := range ldapUpstreams { upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) } + for _, idp := range adUpstreams { + upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) + } plog.Warning("Too many upstream providers are configured (found: %s)", upstreamIDPNames) return nil, nil, httperr.New( http.StatusUnprocessableEntity, @@ -285,6 +289,8 @@ func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider ) case len(oidcUpstreams) == 1: return oidcUpstreams[0], nil, nil + case len(adUpstreams) == 1: + return nil, adUpstreams[0], nil default: return nil, ldapUpstreams[0], nil } diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index e6917954..9a4a99ef 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -397,6 +397,27 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, }, + { + name: "ActiveDirectory upstream happy path using GET", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(happyLDAPUsername), + customPasswordHeader: pointer.StringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, + wantBodyStringWithLocationInHref: false, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, + wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, + wantDownstreamIDTokenGroups: happyLDAPGroups, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + }, { name: "OIDC upstream happy path using GET with a CSRF cookie", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), diff --git a/internal/oidc/idpdiscovery/idp_discovery_handler.go b/internal/oidc/idpdiscovery/idp_discovery_handler.go index 9ee0bf76..8bb6f005 100644 --- a/internal/oidc/idpdiscovery/idp_discovery_handler.go +++ b/internal/oidc/idpdiscovery/idp_discovery_handler.go @@ -14,8 +14,9 @@ import ( ) const ( - idpDiscoveryTypeLDAP = "ldap" - idpDiscoveryTypeOIDC = "oidc" + idpDiscoveryTypeLDAP = "ldap" + idpDiscoveryTypeOIDC = "oidc" + idpDiscoveryTypeActiveDirectory = "activedirectory" ) type response struct { @@ -58,6 +59,9 @@ func responseAsJSON(upstreamIDPs oidc.UpstreamIdentityProvidersLister) ([]byte, for _, provider := range upstreamIDPs.GetLDAPIdentityProviders() { r.IDPs = append(r.IDPs, identityProviderResponse{Name: provider.GetName(), Type: idpDiscoveryTypeLDAP}) } + for _, provider := range upstreamIDPs.GetActiveDirectoryIdentityProviders() { + r.IDPs = append(r.IDPs, identityProviderResponse{Name: provider.GetName(), Type: idpDiscoveryTypeActiveDirectory}) + } for _, provider := range upstreamIDPs.GetOIDCIdentityProviders() { r.IDPs = append(r.IDPs, identityProviderResponse{Name: provider.GetName(), Type: idpDiscoveryTypeOIDC}) } diff --git a/internal/oidc/idpdiscovery/idp_discovery_handler_test.go b/internal/oidc/idpdiscovery/idp_discovery_handler_test.go index 3912f9c9..f4836818 100644 --- a/internal/oidc/idpdiscovery/idp_discovery_handler_test.go +++ b/internal/oidc/idpdiscovery/idp_discovery_handler_test.go @@ -41,12 +41,14 @@ func TestIDPDiscovery(t *testing.T) { {Name: "a-some-oidc-idp", Type: "oidc"}, {Name: "x-some-idp", Type: "ldap"}, {Name: "x-some-idp", Type: "oidc"}, + {Name: "z-some-ad-idp", Type: "activedirectory"}, {Name: "z-some-ldap-idp", Type: "ldap"}, {Name: "z-some-oidc-idp", Type: "oidc"}, }, }, wantSecondResponseBodyJSON: &response{ IDPs: []identityProviderResponse{ + {Name: "some-other-ad-idp-1", Type: "activedirectory"}, {Name: "some-other-ldap-idp-1", Type: "ldap"}, {Name: "some-other-ldap-idp-2", Type: "ldap"}, {Name: "some-other-oidc-idp-1", Type: "oidc"}, @@ -73,6 +75,7 @@ func TestIDPDiscovery(t *testing.T) { WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "a-some-oidc-idp"}). WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "z-some-ldap-idp"}). WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "x-some-idp"}). + WithActiveDirectory(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "z-some-ad-idp"}). Build() handler := NewHandler(idpLister) @@ -104,6 +107,10 @@ func TestIDPDiscovery(t *testing.T) { &oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "some-other-oidc-idp-2"}, }) + idpLister.SetActiveDirectoryIdentityProviders([]provider.UpstreamLDAPIdentityProviderI{ + &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ad-idp-1"}, + }) + // Make the same request to the same handler instance again, and expect different results. rsp = httptest.NewRecorder() handler.ServeHTTP(rsp, req) diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index d29979c8..e5038689 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -267,9 +267,14 @@ type UpstreamLDAPIdentityProvidersLister interface { GetLDAPIdentityProviders() []provider.UpstreamLDAPIdentityProviderI } +type UpstreamActiveDirectoryIdentityProviderLister interface { + GetActiveDirectoryIdentityProviders() []provider.UpstreamLDAPIdentityProviderI +} + type UpstreamIdentityProvidersLister interface { UpstreamOIDCIdentityProvidersLister UpstreamLDAPIdentityProvidersLister + UpstreamActiveDirectoryIdentityProviderLister } func GrantScopeIfRequested(authorizeRequester fosite.AuthorizeRequester, scopeName string) { diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index af24e724..5f0c39f1 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -67,18 +67,22 @@ type DynamicUpstreamIDPProvider interface { GetOIDCIdentityProviders() []UpstreamOIDCIdentityProviderI SetLDAPIdentityProviders(ldapIDPs []UpstreamLDAPIdentityProviderI) GetLDAPIdentityProviders() []UpstreamLDAPIdentityProviderI + SetActiveDirectoryIdentityProviders(adIDPs []UpstreamLDAPIdentityProviderI) + GetActiveDirectoryIdentityProviders() []UpstreamLDAPIdentityProviderI } type dynamicUpstreamIDPProvider struct { - oidcUpstreams []UpstreamOIDCIdentityProviderI - ldapUpstreams []UpstreamLDAPIdentityProviderI - mutex sync.RWMutex + oidcUpstreams []UpstreamOIDCIdentityProviderI + ldapUpstreams []UpstreamLDAPIdentityProviderI + activeDirectoryUpstreams []UpstreamLDAPIdentityProviderI + mutex sync.RWMutex } func NewDynamicUpstreamIDPProvider() DynamicUpstreamIDPProvider { return &dynamicUpstreamIDPProvider{ - oidcUpstreams: []UpstreamOIDCIdentityProviderI{}, - ldapUpstreams: []UpstreamLDAPIdentityProviderI{}, + oidcUpstreams: []UpstreamOIDCIdentityProviderI{}, + ldapUpstreams: []UpstreamLDAPIdentityProviderI{}, + activeDirectoryUpstreams: []UpstreamLDAPIdentityProviderI{}, } } @@ -105,3 +109,15 @@ func (p *dynamicUpstreamIDPProvider) GetLDAPIdentityProviders() []UpstreamLDAPId defer p.mutex.RUnlock() return p.ldapUpstreams } + +func (p *dynamicUpstreamIDPProvider) SetActiveDirectoryIdentityProviders(adIDPs []UpstreamLDAPIdentityProviderI) { + p.mutex.Lock() // acquire a write lock + defer p.mutex.Unlock() + p.activeDirectoryUpstreams = adIDPs +} + +func (p *dynamicUpstreamIDPProvider) GetActiveDirectoryIdentityProviders() []UpstreamLDAPIdentityProviderI { + p.mutex.RLock() // acquire a read lock + defer p.mutex.RUnlock() + return p.activeDirectoryUpstreams +} diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index f690af52..9bed018e 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -148,8 +148,9 @@ func (u *TestUpstreamOIDCIdentityProvider) ValidateToken(_ context.Context, _ *o } type UpstreamIDPListerBuilder struct { - upstreamOIDCIdentityProviders []*TestUpstreamOIDCIdentityProvider - upstreamLDAPIdentityProviders []*TestUpstreamLDAPIdentityProvider + upstreamOIDCIdentityProviders []*TestUpstreamOIDCIdentityProvider + upstreamLDAPIdentityProviders []*TestUpstreamLDAPIdentityProvider + upstreamActiveDirectoryIdentityProviders []*TestUpstreamLDAPIdentityProvider } func (b *UpstreamIDPListerBuilder) WithOIDC(upstreamOIDCIdentityProviders ...*TestUpstreamOIDCIdentityProvider) *UpstreamIDPListerBuilder { @@ -162,6 +163,11 @@ func (b *UpstreamIDPListerBuilder) WithLDAP(upstreamLDAPIdentityProviders ...*Te return b } +func (b *UpstreamIDPListerBuilder) WithActiveDirectory(upstreamActiveDirectoryIdentityProviders ...*TestUpstreamLDAPIdentityProvider) *UpstreamIDPListerBuilder { + b.upstreamActiveDirectoryIdentityProviders = append(b.upstreamActiveDirectoryIdentityProviders, upstreamActiveDirectoryIdentityProviders...) + return b +} + func (b *UpstreamIDPListerBuilder) Build() provider.DynamicUpstreamIDPProvider { idpProvider := provider.NewDynamicUpstreamIDPProvider() @@ -177,6 +183,12 @@ func (b *UpstreamIDPListerBuilder) Build() provider.DynamicUpstreamIDPProvider { } idpProvider.SetLDAPIdentityProviders(ldapUpstreams) + adUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, len(b.upstreamActiveDirectoryIdentityProviders)) + for i := range b.upstreamActiveDirectoryIdentityProviders { + adUpstreams[i] = provider.UpstreamLDAPIdentityProviderI(b.upstreamActiveDirectoryIdentityProviders[i]) + } + idpProvider.SetActiveDirectoryIdentityProviders(adUpstreams) + return idpProvider } diff --git a/internal/upstreamad/upstreamad.go b/internal/upstreamad/upstreamad.go index eff0a9f9..37fe3912 100644 --- a/internal/upstreamad/upstreamad.go +++ b/internal/upstreamad/upstreamad.go @@ -17,6 +17,8 @@ import ( "strings" "time" + "go.pinniped.dev/internal/upstreamldap" + "github.com/go-ldap/ldap/v3" "github.com/gofrs/uuid" "k8s.io/apiserver/pkg/authentication/authenticator" @@ -40,74 +42,6 @@ const ( defaultLDAPSPort = uint16(636) ) -// Conn abstracts the upstream LDAP communication protocol (mostly for testing). -type Conn interface { - Bind(username, password string) error - - Search(searchRequest *ldap.SearchRequest) (*ldap.SearchResult, error) - - SearchWithPaging(searchRequest *ldap.SearchRequest, pagingSize uint32) (*ldap.SearchResult, error) - - Close() -} - -// Our Conn type is subset of the ldap.Client interface, which is implemented by ldap.Conn. -var _ Conn = &ldap.Conn{} - -// LDAPDialer is a factory of Conn, and the resulting Conn can then be used to interact with an upstream LDAP IDP. -type LDAPDialer interface { - Dial(ctx context.Context, addr endpointaddr.HostPort) (Conn, error) -} - -// LDAPDialerFunc makes it easy to use a func as an LDAPDialer. -type LDAPDialerFunc func(ctx context.Context, addr endpointaddr.HostPort) (Conn, error) - -var _ LDAPDialer = LDAPDialerFunc(nil) - -func (f LDAPDialerFunc) Dial(ctx context.Context, addr endpointaddr.HostPort) (Conn, error) { - return f(ctx, addr) -} - -type LDAPConnectionProtocol string - -const ( - StartTLS = LDAPConnectionProtocol("StartTLS") - TLS = LDAPConnectionProtocol("TLS") -) - -// ProviderConfig includes all of the settings for connection and searching for users and groups in -// the upstream LDAP IDP. It also provides methods for testing the connection and performing logins. -// The nested structs are not pointer fields to enable deep copy on function params and return values. -type ProviderConfig struct { - // Name is the unique name of this upstream LDAP IDP. - Name string - - // Host is the hostname or "hostname:port" of the LDAP server. When the port is not specified, - // the default LDAP port will be used. - Host string - - // ConnectionProtocol determines how to establish the connection to the server. Either StartTLS or TLS. - ConnectionProtocol LDAPConnectionProtocol - - // PEM-encoded CA cert bundle to trust when connecting to the LDAP server. Can be nil. - CABundle []byte - - // BindUsername is the username to use when performing a bind with the upstream active directory IDP. - BindUsername string - - // BindPassword is the password to use when performing a bind with the upstream active directory IDP. - BindPassword string - - // UserSearch contains information about how to search for users in the upstream active directory IDP. - UserSearch UserSearchConfig - - // GroupSearch contains information about how to search for group membership in the upstream active directory IDP. - GroupSearch GroupSearchConfig - - // Dialer exists to enable testing. When nil, will use a default appropriate for production use. - Dialer LDAPDialer -} - // UserSearchConfig contains information about how to search for users in the upstream active directory IDP. type UserSearchConfig struct { // Base is the base DN to use for the user search in the upstream active directory IDP. @@ -140,7 +74,7 @@ type GroupSearchConfig struct { } type Provider struct { - c ProviderConfig + c upstreamldap.ProviderConfig } var _ provider.UpstreamLDAPIdentityProviderI = &Provider{} @@ -148,16 +82,16 @@ var _ authenticators.UserAuthenticator = &Provider{} // Create a Provider. The config is not a pointer to ensure that a copy of the config is created, // making the resulting Provider use an effectively read-only configuration. -func New(config ProviderConfig) *Provider { +func New(config upstreamldap.ProviderConfig) *Provider { return &Provider{c: config} } // A reader for the config. Returns a copy of the config to keep the underlying config read-only. -func (p *Provider) GetConfig() ProviderConfig { +func (p *Provider) GetConfig() upstreamldap.ProviderConfig { return p.c } -func (p *Provider) dial(ctx context.Context) (Conn, error) { +func (p *Provider) dial(ctx context.Context) (upstreamldap.Conn, error) { tlsAddr, err := endpointaddr.Parse(p.c.Host, defaultLDAPSPort) if err != nil { return nil, ldap.NewError(ldap.ErrorNetwork, err) @@ -169,13 +103,13 @@ func (p *Provider) dial(ctx context.Context) (Conn, error) { } // Choose how and where to dial based on TLS vs. StartTLS config option. - var dialFunc LDAPDialerFunc + var dialFunc upstreamldap.LDAPDialerFunc var addr endpointaddr.HostPort switch { - case p.c.ConnectionProtocol == TLS: + case p.c.ConnectionProtocol == upstreamldap.TLS: dialFunc = p.dialTLS addr = tlsAddr - case p.c.ConnectionProtocol == StartTLS: + case p.c.ConnectionProtocol == upstreamldap.StartTLS: dialFunc = p.dialStartTLS addr = startTLSAddr default: @@ -193,7 +127,7 @@ func (p *Provider) dial(ctx context.Context) (Conn, error) { // dialTLS is a default implementation of the Dialer, used when Dialer is nil and ConnectionProtocol is TLS. // Unfortunately, the go-ldap library does not seem to support dialing with a context.Context, // so we implement it ourselves, heavily inspired by ldap.DialURL. -func (p *Provider) dialTLS(ctx context.Context, addr endpointaddr.HostPort) (Conn, error) { +func (p *Provider) dialTLS(ctx context.Context, addr endpointaddr.HostPort) (upstreamldap.Conn, error) { tlsConfig, err := p.tlsConfig() if err != nil { return nil, ldap.NewError(ldap.ErrorNetwork, err) @@ -213,7 +147,7 @@ func (p *Provider) dialTLS(ctx context.Context, addr endpointaddr.HostPort) (Con // dialTLS is a default implementation of the Dialer, used when Dialer is nil and ConnectionProtocol is StartTLS. // Unfortunately, the go-ldap library does not seem to support dialing with a context.Context, // so we implement it ourselves, heavily inspired by ldap.DialURL. -func (p *Provider) dialStartTLS(ctx context.Context, addr endpointaddr.HostPort) (Conn, error) { +func (p *Provider) dialStartTLS(ctx context.Context, addr endpointaddr.HostPort) (upstreamldap.Conn, error) { tlsConfig, err := p.tlsConfig() if err != nil { return nil, ldap.NewError(ldap.ErrorNetwork, err) @@ -295,7 +229,7 @@ func (p *Provider) TestConnection(ctx context.Context) error { // not bind as that user, so it does not test their password. It returns the same values that a real call to // AuthenticateUser with the correct password would return. func (p *Provider) DryRunAuthenticateUser(ctx context.Context, username string) (*authenticator.Response, bool, error) { - endUserBindFunc := func(conn Conn, foundUserDN string) error { + endUserBindFunc := func(conn upstreamldap.Conn, foundUserDN string) error { // Act as if the end user bind always succeeds. return nil } @@ -304,13 +238,13 @@ func (p *Provider) DryRunAuthenticateUser(ctx context.Context, username string) // Authenticate an end user and return their mapped username, groups, and UID. Implements authenticators.UserAuthenticator. func (p *Provider) AuthenticateUser(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { - endUserBindFunc := func(conn Conn, foundUserDN string) error { + endUserBindFunc := func(conn upstreamldap.Conn, foundUserDN string) error { return conn.Bind(foundUserDN, password) } return p.authenticateUserImpl(ctx, username, endUserBindFunc) } -func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticator.Response, bool, error) { +func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bindFunc func(conn upstreamldap.Conn, foundUserDN string) error) (*authenticator.Response, bool, error) { t := trace.FromContext(ctx).Nest("slow ldap authenticate user attempt", trace.Field{Key: "providerName", Value: p.GetName()}) defer t.LogIfLong(500 * time.Millisecond) // to help users debug slow LDAP searches @@ -361,7 +295,7 @@ func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bi return response, true, nil } -func (p *Provider) searchGroupsForUserDN(conn Conn, userDN string) ([]string, error) { +func (p *Provider) searchGroupsForUserDN(conn upstreamldap.Conn, userDN string) ([]string, error) { searchResult, err := conn.SearchWithPaging(p.groupSearchRequest(userDN), groupSearchPageSize) if err != nil { return nil, fmt.Errorf(`error searching for group memberships for user with DN %q: %w`, userDN, err) @@ -396,7 +330,7 @@ func (p *Provider) validateConfig() error { return nil } -func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(conn Conn, foundUserDN string) error) (string, string, []string, error) { +func (p *Provider) searchAndBindUser(conn upstreamldap.Conn, username string, bindFunc func(conn upstreamldap.Conn, foundUserDN string) error) (string, string, []string, error) { searchResult, err := conn.Search(p.userSearchRequest(username)) if err != nil { plog.All(`error searching for user`, diff --git a/internal/upstreamad/upstreamad_test.go b/internal/upstreamad/upstreamad_test.go index a518bae2..1fe0fca8 100644 --- a/internal/upstreamad/upstreamad_test.go +++ b/internal/upstreamad/upstreamad_test.go @@ -15,6 +15,8 @@ import ( "testing" "time" + "go.pinniped.dev/internal/upstreamldap" + "github.com/go-ldap/ldap/v3" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -57,21 +59,21 @@ var ( ) func TestEndUserAuthentication(t *testing.T) { - providerConfig := func(editFunc func(p *ProviderConfig)) *ProviderConfig { - config := &ProviderConfig{ + providerConfig := func(editFunc func(p *upstreamldap.ProviderConfig)) *upstreamldap.ProviderConfig { + config := &upstreamldap.ProviderConfig{ Name: "some-provider-name", Host: testHost, CABundle: nil, // this field is only used by the production dialer, which is replaced by a mock for this test - ConnectionProtocol: TLS, + ConnectionProtocol: upstreamldap.TLS, BindUsername: testBindUsername, BindPassword: testBindPassword, - UserSearch: UserSearchConfig{ + UserSearch: upstreamldap.UserSearchConfig{ Base: testUserSearchBase, Filter: testUserSearchFilter, UsernameAttribute: testUserSearchUsernameAttribute, UIDAttribute: testUserSearchUIDAttribute, }, - GroupSearch: GroupSearchConfig{ + GroupSearch: upstreamldap.GroupSearchConfig{ Base: testGroupSearchBase, Filter: testGroupSearchFilter, GroupNameAttribute: testGroupSearchGroupNameAttribute, @@ -167,7 +169,7 @@ func TestEndUserAuthentication(t *testing.T) { name string username string password string - providerConfig *ProviderConfig + providerConfig *upstreamldap.ProviderConfig searchMocks func(conn *mockldapconn.MockConn) bindEndUserMocks func(conn *mockldapconn.MockConn) dialError error @@ -198,14 +200,14 @@ func TestEndUserAuthentication(t *testing.T) { name: "default as much as possible", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: &ProviderConfig{ + providerConfig: &upstreamldap.ProviderConfig{ Name: "some-provider-name", Host: testHost, CABundle: nil, // this field is only used by the production dialer, which is replaced by a mock for this test - ConnectionProtocol: TLS, + ConnectionProtocol: upstreamldap.TLS, BindUsername: testBindUsername, BindPassword: testBindPassword, - GroupSearch: GroupSearchConfig{ + GroupSearch: upstreamldap.GroupSearchConfig{ Base: testGroupSearchBase, Filter: testGroupSearchFilter, GroupNameAttribute: testGroupSearchGroupNameAttribute, @@ -241,7 +243,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the user search filter is already wrapped by parenthesis then it is not wrapped again", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Filter = "(" + testUserSearchFilter + ")" }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -260,7 +262,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the group search filter is already wrapped by parenthesis then it is not wrapped again", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.GroupSearch.Filter = "(" + testGroupSearchFilter + ")" }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -279,7 +281,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the group search base is empty then skip the group search entirely", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.GroupSearch.Base = "" // this configuration means that the user does not want group search to happen }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -298,7 +300,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the UsernameAttribute is dn and there is a user search filter provided", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UsernameAttribute = "dn" }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -330,7 +332,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the UIDAttribute is dn", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UIDAttribute = "dn" }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -362,7 +364,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the GroupNameAttribute is empty then it defaults to dn", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.GroupSearch.GroupNameAttribute = "" // blank means to use dn }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -385,7 +387,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the GroupNameAttribute is dn", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.GroupSearch.GroupNameAttribute = "dn" }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -408,7 +410,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the GroupNameAttribute is cn", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.GroupSearch.GroupNameAttribute = "cn" }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -446,7 +448,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when user search Filter is blank it derives a search filter from the UsernameAttribute", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Filter = "" }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -467,7 +469,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when user search Filter and user attribute is blank it defaults to sAMAccountName={}", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Filter = "" p.UserSearch.UsernameAttribute = "" }), @@ -500,7 +502,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when group search Filter is blank it uses a default search filter of member={}", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.GroupSearch.Filter = "" }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -594,7 +596,7 @@ func TestEndUserAuthentication(t *testing.T) { name: "when the UsernameAttribute is dn and there is not a user search filter provided", username: testUpstreamUsername, password: testUpstreamPassword, - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UsernameAttribute = "dn" p.UserSearch.Filter = "" }), @@ -1027,7 +1029,7 @@ func TestEndUserAuthentication(t *testing.T) { } dialWasAttempted := false - tt.providerConfig.Dialer = LDAPDialerFunc(func(ctx context.Context, addr endpointaddr.HostPort) (Conn, error) { + tt.providerConfig.Dialer = upstreamldap.LDAPDialerFunc(func(ctx context.Context, addr endpointaddr.HostPort) (upstreamldap.Conn, error) { dialWasAttempted = true require.Equal(t, tt.providerConfig.Host, addr.Endpoint()) if tt.dialError != nil { @@ -1091,15 +1093,15 @@ func TestEndUserAuthentication(t *testing.T) { } func TestTestConnection(t *testing.T) { - providerConfig := func(editFunc func(p *ProviderConfig)) *ProviderConfig { - config := &ProviderConfig{ + providerConfig := func(editFunc func(p *upstreamldap.ProviderConfig)) *upstreamldap.ProviderConfig { + config := &upstreamldap.ProviderConfig{ Name: "some-provider-name", Host: testHost, CABundle: nil, // this field is only used by the production dialer, which is replaced by a mock for this test - ConnectionProtocol: TLS, + ConnectionProtocol: upstreamldap.TLS, BindUsername: testBindUsername, BindPassword: testBindPassword, - UserSearch: UserSearchConfig{}, // not used by TestConnection + UserSearch: upstreamldap.UserSearchConfig{}, // not used by TestConnection } if editFunc != nil { editFunc(config) @@ -1109,7 +1111,7 @@ func TestTestConnection(t *testing.T) { tests := []struct { name string - providerConfig *ProviderConfig + providerConfig *upstreamldap.ProviderConfig setupMocks func(conn *mockldapconn.MockConn) dialError error wantError string @@ -1140,7 +1142,7 @@ func TestTestConnection(t *testing.T) { }, { name: "when the config is invalid", - providerConfig: providerConfig(func(p *ProviderConfig) { + providerConfig: providerConfig(func(p *upstreamldap.ProviderConfig) { // This particular combination of options is not allowed. p.UserSearch.UsernameAttribute = "dn" p.UserSearch.Filter = "" @@ -1162,7 +1164,7 @@ func TestTestConnection(t *testing.T) { } dialWasAttempted := false - tt.providerConfig.Dialer = LDAPDialerFunc(func(ctx context.Context, addr endpointaddr.HostPort) (Conn, error) { + tt.providerConfig.Dialer = upstreamldap.LDAPDialerFunc(func(ctx context.Context, addr endpointaddr.HostPort) (upstreamldap.Conn, error) { dialWasAttempted = true require.Equal(t, tt.providerConfig.Host, addr.Endpoint()) if tt.dialError != nil { @@ -1187,13 +1189,13 @@ func TestTestConnection(t *testing.T) { } func TestGetConfig(t *testing.T) { - c := ProviderConfig{ + c := upstreamldap.ProviderConfig{ Name: "original-provider-name", Host: testHost, CABundle: []byte("some-ca-bundle"), BindUsername: testBindUsername, BindPassword: testBindPassword, - UserSearch: UserSearchConfig{ + UserSearch: upstreamldap.UserSearchConfig{ Base: testUserSearchBase, Filter: testUserSearchFilter, UsernameAttribute: testUserSearchUsernameAttribute, @@ -1217,16 +1219,16 @@ func TestGetConfig(t *testing.T) { func TestGetURL(t *testing.T) { require.Equal(t, "ldaps://ldap.example.com:1234?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev", - New(ProviderConfig{ + New(upstreamldap.ProviderConfig{ Host: "ldap.example.com:1234", - UserSearch: UserSearchConfig{Base: "ou=users,dc=pinniped,dc=dev"}, + UserSearch: upstreamldap.UserSearchConfig{Base: "ou=users,dc=pinniped,dc=dev"}, }).GetURL().String()) require.Equal(t, "ldaps://ldap.example.com?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev", - New(ProviderConfig{ + New(upstreamldap.ProviderConfig{ Host: "ldap.example.com", - UserSearch: UserSearchConfig{Base: "ou=users,dc=pinniped,dc=dev"}, + UserSearch: upstreamldap.UserSearchConfig{Base: "ou=users,dc=pinniped,dc=dev"}, }).GetURL().String()) } @@ -1255,7 +1257,7 @@ func TestRealTLSDialing(t *testing.T) { tests := []struct { name string host string - connProto LDAPConnectionProtocol + connProto upstreamldap.LDAPConnectionProtocol caBundle []byte context context.Context wantError string @@ -1264,14 +1266,14 @@ func TestRealTLSDialing(t *testing.T) { name: "happy path", host: testServerHostAndPort, caBundle: []byte(testServerCABundle), - connProto: TLS, + connProto: upstreamldap.TLS, context: context.Background(), }, { name: "server cert name does not match the address to which the client connected", host: testServerWithBadCertNameAddr, caBundle: caForTestServerWithBadCertName.Bundle(), - connProto: TLS, + connProto: upstreamldap.TLS, context: context.Background(), wantError: `LDAP Result Code 200 "Network Error": x509: certificate is valid for 10.2.3.4, not 127.0.0.1`, }, @@ -1279,7 +1281,7 @@ func TestRealTLSDialing(t *testing.T) { name: "invalid CA bundle with TLS", host: testServerHostAndPort, caBundle: []byte("not a ca bundle"), - connProto: TLS, + connProto: upstreamldap.TLS, context: context.Background(), wantError: `LDAP Result Code 200 "Network Error": could not parse CA bundle`, }, @@ -1287,7 +1289,7 @@ func TestRealTLSDialing(t *testing.T) { name: "invalid CA bundle with StartTLS", host: testServerHostAndPort, caBundle: []byte("not a ca bundle"), - connProto: StartTLS, + connProto: upstreamldap.StartTLS, context: context.Background(), wantError: `LDAP Result Code 200 "Network Error": could not parse CA bundle`, }, @@ -1295,7 +1297,7 @@ func TestRealTLSDialing(t *testing.T) { name: "invalid host with TLS", host: "this:is:not:a:valid:hostname", caBundle: []byte(testServerCABundle), - connProto: TLS, + connProto: upstreamldap.TLS, context: context.Background(), wantError: `LDAP Result Code 200 "Network Error": host "this:is:not:a:valid:hostname" is not a valid hostname or IP address`, }, @@ -1303,7 +1305,7 @@ func TestRealTLSDialing(t *testing.T) { name: "invalid host with StartTLS", host: "this:is:not:a:valid:hostname", caBundle: []byte(testServerCABundle), - connProto: StartTLS, + connProto: upstreamldap.StartTLS, context: context.Background(), wantError: `LDAP Result Code 200 "Network Error": host "this:is:not:a:valid:hostname" is not a valid hostname or IP address`, }, @@ -1311,7 +1313,7 @@ func TestRealTLSDialing(t *testing.T) { name: "missing CA bundle when it is required because the host is not using a trusted CA", host: testServerHostAndPort, caBundle: nil, - connProto: TLS, + connProto: upstreamldap.TLS, context: context.Background(), wantError: `LDAP Result Code 200 "Network Error": x509: certificate signed by unknown authority`, }, @@ -1320,7 +1322,7 @@ func TestRealTLSDialing(t *testing.T) { // This is assuming that this port was not reclaimed by another app since the test setup ran. Seems safe enough. host: recentlyClaimedHostAndPort, caBundle: []byte(testServerCABundle), - connProto: TLS, + connProto: upstreamldap.TLS, context: context.Background(), wantError: fmt.Sprintf(`LDAP Result Code 200 "Network Error": dial tcp %s: connect: connection refused`, recentlyClaimedHostAndPort), }, @@ -1328,7 +1330,7 @@ func TestRealTLSDialing(t *testing.T) { name: "pays attention to the passed context", host: testServerHostAndPort, caBundle: []byte(testServerCABundle), - connProto: TLS, + connProto: upstreamldap.TLS, context: alreadyCancelledContext, wantError: fmt.Sprintf(`LDAP Result Code 200 "Network Error": dial tcp %s: operation was canceled`, testServerHostAndPort), }, @@ -1344,7 +1346,7 @@ func TestRealTLSDialing(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - provider := New(ProviderConfig{ + provider := New(upstreamldap.ProviderConfig{ Host: tt.host, CABundle: tt.caBundle, ConnectionProtocol: tt.connProto,