From 5d8d7246c2744d34e81399f0d9b120adfc034959 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Mon, 19 Jul 2021 13:54:07 -0700 Subject: [PATCH] Refactor active directory and ldap controllers to share almost everything Signed-off-by: Ryan Richard --- .../active_directory_upstream_watcher.go | 339 ++++++------------ .../active_directory_upstream_watcher_test.go | 69 ++-- .../ldap_upstream_watcher.go | 331 +++++------------ .../ldap_upstream_watcher_test.go | 41 +-- .../upstreamwatchers/upstream_watchers.go | 307 +++++++++++++++- test/integration/supervisor_login_test.go | 4 +- 6 files changed, 562 insertions(+), 529 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index be69e76e..27357bfe 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -6,12 +6,8 @@ package activedirectoryupstreamwatcher import ( "context" - "crypto/x509" - "encoding/base64" "fmt" - "time" - 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" @@ -26,28 +22,109 @@ import ( "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" + activeDirectoryControllerName = "active-directory-upstream-observer" // Default values for active directory config. defaultActiveDirectoryUsernameAttributeName = "sAMAccountName" defaultActiveDirectoryUIDAttributeName = "objectGUID" ) +type activeDirectoryUpstreamGenericLDAPImpl struct { + activeDirectoryIdentityProvider v1alpha1.ActiveDirectoryIdentityProvider +} + +func (g *activeDirectoryUpstreamGenericLDAPImpl) Spec() upstreamwatchers.UpstreamGenericLDAPSpec { + return &activeDirectoryUpstreamGenericLDAPSpec{g.activeDirectoryIdentityProvider} +} + +func (g *activeDirectoryUpstreamGenericLDAPImpl) Namespace() string { + return g.activeDirectoryIdentityProvider.Namespace +} + +func (g *activeDirectoryUpstreamGenericLDAPImpl) Name() string { + return g.activeDirectoryIdentityProvider.Name +} + +func (g *activeDirectoryUpstreamGenericLDAPImpl) Generation() int64 { + return g.activeDirectoryIdentityProvider.Generation +} + +func (g *activeDirectoryUpstreamGenericLDAPImpl) Status() upstreamwatchers.UpstreamGenericLDAPStatus { + return &activeDirectoryUpstreamGenericLDAPStatus{g.activeDirectoryIdentityProvider} +} + +type activeDirectoryUpstreamGenericLDAPSpec struct { + activeDirectoryIdentityProvider v1alpha1.ActiveDirectoryIdentityProvider +} + +func (s *activeDirectoryUpstreamGenericLDAPSpec) Host() string { + return s.activeDirectoryIdentityProvider.Spec.Host +} + +func (s *activeDirectoryUpstreamGenericLDAPSpec) TLSSpec() *v1alpha1.TLSSpec { + return s.activeDirectoryIdentityProvider.Spec.TLS +} + +func (s *activeDirectoryUpstreamGenericLDAPSpec) BindSecretName() string { + return s.activeDirectoryIdentityProvider.Spec.Bind.SecretName +} + +func (s *activeDirectoryUpstreamGenericLDAPSpec) UserSearch() upstreamwatchers.UpstreamGenericLDAPUserSearch { + return &activeDirectoryUpstreamGenericLDAPUserSearch{s.activeDirectoryIdentityProvider.Spec.UserSearch} +} + +func (s *activeDirectoryUpstreamGenericLDAPSpec) GroupSearch() upstreamwatchers.UpstreamGenericLDAPGroupSearch { + return &activeDirectoryUpstreamGenericLDAPGroupSearch{s.activeDirectoryIdentityProvider.Spec.GroupSearch} +} + +type activeDirectoryUpstreamGenericLDAPUserSearch struct { + userSearch v1alpha1.ActiveDirectoryIdentityProviderUserSearch +} + +func (u *activeDirectoryUpstreamGenericLDAPUserSearch) Base() string { + return u.userSearch.Base +} + +func (u *activeDirectoryUpstreamGenericLDAPUserSearch) Filter() string { + return u.userSearch.Filter +} + +func (u *activeDirectoryUpstreamGenericLDAPUserSearch) UsernameAttribute() string { + return u.userSearch.Attributes.Username +} + +func (u *activeDirectoryUpstreamGenericLDAPUserSearch) UIDAttribute() string { + return u.userSearch.Attributes.UID +} + +type activeDirectoryUpstreamGenericLDAPGroupSearch struct { + groupSearch v1alpha1.ActiveDirectoryIdentityProviderGroupSearch +} + +func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) Base() string { + return g.groupSearch.Base +} + +func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) Filter() string { + return g.groupSearch.Filter +} + +func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) GroupNameAttribute() string { + return g.groupSearch.Attributes.GroupName +} + +type activeDirectoryUpstreamGenericLDAPStatus struct { + activeDirectoryIdentityProvider v1alpha1.ActiveDirectoryIdentityProvider +} + +func (s *activeDirectoryUpstreamGenericLDAPStatus) Conditions() []v1alpha1.Condition { + return s.activeDirectoryIdentityProvider.Status.Conditions +} + // UpstreamActiveDirectoryIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. type UpstreamActiveDirectoryIdentityProviderICache interface { SetActiveDirectoryIdentityProviders([]provider.UpstreamLDAPIdentityProviderI) @@ -55,28 +132,13 @@ type UpstreamActiveDirectoryIdentityProviderICache interface { type activeDirectoryWatcherController struct { cache UpstreamActiveDirectoryIdentityProviderICache - validatedSecretVersionsCache *secretVersionCache + validatedSecretVersionsCache *upstreamwatchers.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, @@ -88,7 +150,7 @@ func New( return newInternal( idpCache, // start with an empty secretVersionCache - newSecretVersionCache(), + upstreamwatchers.NewSecretVersionCache(), // nil means to use a real production dialer when creating objects to add to the cache nil, client, @@ -101,7 +163,7 @@ func New( // For test dependency injection purposes. func newInternal( idpCache UpstreamActiveDirectoryIdentityProviderICache, - validatedSecretVersionsCache *secretVersionCache, + validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache, ldapDialer upstreamldap.LDAPDialer, client pinnipedclientset.Interface, activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer, @@ -125,7 +187,7 @@ func newInternal( ), withInformer( secretInformer, - pinnipedcontroller.MatchAnySecretOfTypeFilter(activeDirectoryBindAccountSecretType, pinnipedcontroller.SingletonQueue()), + pinnipedcontroller.MatchAnySecretOfTypeFilter(upstreamwatchers.LDAPBindAccountSecretType, pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), ) @@ -187,212 +249,11 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, Dialer: c.ldapDialer, } - conditions := []*v1alpha1.Condition{} - secretValidCondition, currentSecretVersion := c.validateSecret(upstream, config) - tlsValidCondition := c.validateTLSConfig(upstream, config) - conditions = append(conditions, secretValidCondition, tlsValidCondition) + conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &activeDirectoryUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.validatedSecretVersionsCache, config) - // 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.Conditions()) - 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 = upstreamldap.New(*config) - // Try again hoping that the condition will improve. - requeue = true - default: - // Fully validated provider, so load it into the cache. - p = upstreamldap.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 + return upstreamwatchers.EvaluateConditions(conditions, config) } func (c *activeDirectoryWatcherController) updateStatus(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider, conditions []*v1alpha1.Condition) { diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index 95468c2a..f944c710 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -25,6 +25,7 @@ import ( 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/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/mocks/mockldapconn" @@ -235,7 +236,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } activeDirectoryConnectionValidTrueCondition := func(gen int64, secretVersion string) v1alpha1.Condition { return v1alpha1.Condition{ - Type: "ActiveDirectoryConnectionValid", + Type: "LDAPConnectionValid", Status: "True", LastTransitionTime: now, Reason: "Success", @@ -257,8 +258,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } allConditionsTrue := func(gen int64, secretVersion string) []v1alpha1.Condition { return []v1alpha1.Condition{ - activeDirectoryConnectionValidTrueCondition(gen, secretVersion), bindSecretValidTrueCondition(gen), + activeDirectoryConnectionValidTrueCondition(gen, secretVersion), tlsConfigurationValidLoadedTrueCondition(gen), } } @@ -273,7 +274,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { tests := []struct { name string - initialValidatedSettings map[string]validatedSettings + initialValidatedSettings map[string]upstreamwatchers.ValidatedSettings inputUpstreams []runtime.Object inputSecrets []runtime.Object setupMocks func(conn *mockldapconn.MockConn) @@ -281,7 +282,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { wantErr string wantResultingCache []*upstreamldap.ProviderConfig wantResultingUpstreams []v1alpha1.ActiveDirectoryIdentityProvider - wantValidatedSettings map[string]validatedSettings + wantValidatedSettings map[string]upstreamwatchers.ValidatedSettings }{ { name: "no ActiveDirectoryIdentityProvider upstreams clears the cache", @@ -304,7 +305,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, }, { name: "missing secret", @@ -474,8 +475,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ - activeDirectoryConnectionValidTrueCondition(1234, "4242"), bindSecretValidTrueCondition(1234), + activeDirectoryConnectionValidTrueCondition(1234, "4242"), { Type: "TLSConfigurationValid", Status: "True", @@ -487,7 +488,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -530,8 +531,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Ready", Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), { - Type: "ActiveDirectoryConnectionValid", + Type: "LDAPConnectionValid", Status: "True", LastTransitionTime: now, Reason: "Success", @@ -540,12 +542,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { "ldap.example.com", testBindUsername, testSecretName, "4242"), ObservedGeneration: 1234, }, - bindSecretValidTrueCondition(1234), tlsConfigurationValidLoadedTrueCondition(1234), }, }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -587,17 +588,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), { - Type: "ActiveDirectoryConnectionValid", + Type: "LDAPConnectionValid", Status: "False", LastTransitionTime: now, - Reason: "ActiveDirectoryConnectionError", + Reason: "LDAPConnectionError", 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), }, }, @@ -642,7 +643,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, }, { name: "one valid upstream and one invalid upstream updates the cache to include only the valid upstream", @@ -685,7 +686,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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)", @@ -704,17 +705,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ Phase: "Error", Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), { - Type: "ActiveDirectoryConnectionValid", + Type: "LDAPConnectionValid", Status: "False", LastTransitionTime: now, - Reason: "ActiveDirectoryConnectionError", + Reason: "LDAPConnectionError", 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), }, }, @@ -729,7 +730,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + initialValidatedSettings: map[string]upstreamwatchers.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. }, @@ -741,7 +742,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -752,7 +753,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + initialValidatedSettings: map[string]upstreamwatchers.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. }, @@ -764,7 +765,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -775,7 +776,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + initialValidatedSettings: map[string]upstreamwatchers.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) @@ -789,7 +790,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -797,17 +798,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { upstream.Generation = 1234 upstream.Status.Conditions = []v1alpha1.Condition{ { - Type: "ActiveDirectoryConnectionValid", + Type: "LDAPConnectionValid", Status: "False", // failure! LastTransitionTime: now, - Reason: "ActiveDirectoryConnectionError", + Reason: "LDAPConnectionError", Message: "some-error-message", ObservedGeneration: 1234, // same (current) generation! }, } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}}, + initialValidatedSettings: map[string]upstreamwatchers.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) @@ -821,7 +822,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -831,8 +832,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { 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 + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, // newer secret version! + initialValidatedSettings: map[string]upstreamwatchers.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) @@ -846,7 +847,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, }, { name: "when the input activedirectoryidentityprovider leaves user attributes blank, provide default values", @@ -887,7 +888,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, }, } @@ -923,7 +924,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { return conn, nil })} - validatedSecretVersionCache := newSecretVersionCache() + validatedSecretVersionCache := upstreamwatchers.NewSecretVersionCache() if tt.initialValidatedSettings != nil { validatedSecretVersionCache.ValidatedSettingsByName = tt.initialValidatedSettings } @@ -977,7 +978,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { // 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{} + tt.wantValidatedSettings = map[string]upstreamwatchers.ValidatedSettings{} } require.Equal(t, tt.wantValidatedSettings, validatedSecretVersionCache.ValidatedSettingsByName) }) diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index 3f1707b9..968b82a6 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -6,8 +6,6 @@ package ldapupstreamwatcher import ( "context" - "crypto/x509" - "encoding/base64" "fmt" "time" @@ -26,7 +24,6 @@ import ( "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" ) @@ -34,16 +31,100 @@ const ( ldapControllerName = "ldap-upstream-observer" ldapBindAccountSecretType = corev1.SecretTypeBasicAuth testLDAPConnectionTimeout = 90 * time.Second - - // Constants related to conditions. - typeBindSecretValid = "BindSecretValid" - typeTLSConfigurationValid = "TLSConfigurationValid" - typeLDAPConnectionValid = "LDAPConnectionValid" - reasonLDAPConnectionError = "LDAPConnectionError" - noTLSConfigurationMessage = "no TLS configuration provided" - loadedTLSConfigurationMessage = "loaded TLS configuration" ) +type ldapUpstreamGenericLDAPImpl struct { + ldapIdentityProvider v1alpha1.LDAPIdentityProvider +} + +func (g *ldapUpstreamGenericLDAPImpl) Spec() upstreamwatchers.UpstreamGenericLDAPSpec { + return &ldapUpstreamGenericLDAPSpec{g.ldapIdentityProvider} +} + +func (g *ldapUpstreamGenericLDAPImpl) Namespace() string { + return g.ldapIdentityProvider.Namespace +} + +func (g *ldapUpstreamGenericLDAPImpl) Name() string { + return g.ldapIdentityProvider.Name +} + +func (g *ldapUpstreamGenericLDAPImpl) Generation() int64 { + return g.ldapIdentityProvider.Generation +} + +func (g *ldapUpstreamGenericLDAPImpl) Status() upstreamwatchers.UpstreamGenericLDAPStatus { + return &ldapUpstreamGenericLDAPStatus{g.ldapIdentityProvider} +} + +type ldapUpstreamGenericLDAPSpec struct { + ldapIdentityProvider v1alpha1.LDAPIdentityProvider +} + +func (s *ldapUpstreamGenericLDAPSpec) Host() string { + return s.ldapIdentityProvider.Spec.Host +} + +func (s *ldapUpstreamGenericLDAPSpec) TLSSpec() *v1alpha1.TLSSpec { + return s.ldapIdentityProvider.Spec.TLS +} + +func (s *ldapUpstreamGenericLDAPSpec) BindSecretName() string { + return s.ldapIdentityProvider.Spec.Bind.SecretName +} + +func (s *ldapUpstreamGenericLDAPSpec) UserSearch() upstreamwatchers.UpstreamGenericLDAPUserSearch { + return &ldapUpstreamGenericLDAPUserSearch{s.ldapIdentityProvider.Spec.UserSearch} +} + +func (s *ldapUpstreamGenericLDAPSpec) GroupSearch() upstreamwatchers.UpstreamGenericLDAPGroupSearch { + return &ldapUpstreamGenericLDAPGroupSearch{s.ldapIdentityProvider.Spec.GroupSearch} +} + +type ldapUpstreamGenericLDAPUserSearch struct { + userSearch v1alpha1.LDAPIdentityProviderUserSearch +} + +func (u *ldapUpstreamGenericLDAPUserSearch) Base() string { + return u.userSearch.Base +} + +func (u *ldapUpstreamGenericLDAPUserSearch) Filter() string { + return u.userSearch.Filter +} + +func (u *ldapUpstreamGenericLDAPUserSearch) UsernameAttribute() string { + return u.userSearch.Attributes.Username +} + +func (u *ldapUpstreamGenericLDAPUserSearch) UIDAttribute() string { + return u.userSearch.Attributes.UID +} + +type ldapUpstreamGenericLDAPGroupSearch struct { + groupSearch v1alpha1.LDAPIdentityProviderGroupSearch +} + +func (g *ldapUpstreamGenericLDAPGroupSearch) Base() string { + return g.groupSearch.Base +} + +func (g *ldapUpstreamGenericLDAPGroupSearch) Filter() string { + return g.groupSearch.Filter +} + +func (g *ldapUpstreamGenericLDAPGroupSearch) GroupNameAttribute() string { + return g.groupSearch.Attributes.GroupName +} + +type ldapUpstreamGenericLDAPStatus struct { + ldapIdentityProvider v1alpha1.LDAPIdentityProvider +} + +func (s *ldapUpstreamGenericLDAPStatus) Conditions() []v1alpha1.Condition { + return s.ldapIdentityProvider.Status.Conditions +} + // UpstreamLDAPIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. type UpstreamLDAPIdentityProviderICache interface { SetLDAPIdentityProviders([]provider.UpstreamLDAPIdentityProviderI) @@ -51,28 +132,13 @@ type UpstreamLDAPIdentityProviderICache interface { type ldapWatcherController struct { cache UpstreamLDAPIdentityProviderICache - validatedSecretVersionsCache *secretVersionCache + validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache ldapDialer upstreamldap.LDAPDialer client pinnipedclientset.Interface ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer secretInformer corev1informers.SecretInformer } -// An in-memory cache with an entry for each LDAPIdentityProvider, 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 UpstreamLDAPIdentityProviderICache. func New( idpCache UpstreamLDAPIdentityProviderICache, @@ -84,7 +150,7 @@ func New( return newInternal( idpCache, // start with an empty secretVersionCache - newSecretVersionCache(), + upstreamwatchers.NewSecretVersionCache(), // nil means to use a real production dialer when creating objects to add to the cache nil, client, @@ -97,7 +163,7 @@ func New( // For test dependency injection purposes. func newInternal( idpCache UpstreamLDAPIdentityProviderICache, - validatedSecretVersionsCache *secretVersionCache, + validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache, ldapDialer upstreamldap.LDAPDialer, client pinnipedclientset.Interface, ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer, @@ -174,212 +240,11 @@ func (c *ldapWatcherController) validateUpstream(ctx context.Context, upstream * Dialer: c.ldapDialer, } - conditions := []*v1alpha1.Condition{} - secretValidCondition, currentSecretVersion := c.validateSecret(upstream, config) - tlsValidCondition := c.validateTLSConfig(upstream, config) - conditions = append(conditions, secretValidCondition, tlsValidCondition) + conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &ldapUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.validatedSecretVersionsCache, config) - // 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.Conditions()) - 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 = upstreamldap.New(*config) - // Try again hoping that the condition will improve. - requeue = true - default: - // Fully validated provider, so load it into the cache. - p = upstreamldap.New(*config) - requeue = false - } - - return p, requeue -} - -func (c *ldapWatcherController) validateTLSConfig(upstream *v1alpha1.LDAPIdentityProvider, 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 *ldapWatcherController) validateFinishedConfig(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider, config *upstreamldap.ProviderConfig, currentSecretVersion string) *v1alpha1.Condition { - if c.hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream, currentSecretVersion, config) { - return nil - } - - testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, testLDAPConnectionTimeout) - 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 *ldapWatcherController) testConnection( - ctx context.Context, - upstream *v1alpha1.LDAPIdentityProvider, - 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: typeLDAPConnectionValid, - Status: v1alpha1.ConditionFalse, - Reason: reasonLDAPConnectionError, - 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: typeLDAPConnectionValid, - 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 *ldapWatcherController) hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream *v1alpha1.LDAPIdentityProvider, currentSecretVersion string, config *upstreamldap.ProviderConfig) bool { - currentGeneration := upstream.Generation - for _, cond := range upstream.Status.Conditions { - if cond.Type == typeLDAPConnectionValid && 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 *ldapWatcherController) validTLSCondition(message string) *v1alpha1.Condition { - return &v1alpha1.Condition{ - Type: typeTLSConfigurationValid, - Status: v1alpha1.ConditionTrue, - Reason: upstreamwatchers.ReasonSuccess, - Message: message, - } -} - -func (c *ldapWatcherController) invalidTLSCondition(message string) *v1alpha1.Condition { - return &v1alpha1.Condition{ - Type: typeTLSConfigurationValid, - Status: v1alpha1.ConditionFalse, - Reason: upstreamwatchers.ReasonInvalidTLSConfig, - Message: message, - } -} - -func (c *ldapWatcherController) validateSecret(upstream *v1alpha1.LDAPIdentityProvider, 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 + return upstreamwatchers.EvaluateConditions(conditions, config) } func (c *ldapWatcherController) updateStatus(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider, conditions []*v1alpha1.Condition) { diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index c801e741..9d475292 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -25,6 +25,7 @@ import ( 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/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/mocks/mockldapconn" @@ -273,7 +274,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { tests := []struct { name string - initialValidatedSettings map[string]validatedSettings + initialValidatedSettings map[string]upstreamwatchers.ValidatedSettings inputUpstreams []runtime.Object inputSecrets []runtime.Object setupMocks func(conn *mockldapconn.MockConn) @@ -281,7 +282,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { wantErr string wantResultingCache []*upstreamldap.ProviderConfig wantResultingUpstreams []v1alpha1.LDAPIdentityProvider - wantValidatedSettings map[string]validatedSettings + wantValidatedSettings map[string]upstreamwatchers.ValidatedSettings }{ { name: "no LDAPIdentityProvider upstreams clears the cache", @@ -304,7 +305,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, }, { name: "missing secret", @@ -487,7 +488,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -545,7 +546,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -642,7 +643,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, }, { name: "one valid upstream and one invalid upstream updates the cache to include only the valid upstream", @@ -685,7 +686,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, }, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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)", @@ -729,7 +730,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + initialValidatedSettings: map[string]upstreamwatchers.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. }, @@ -741,7 +742,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -752,7 +753,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + initialValidatedSettings: map[string]upstreamwatchers.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. }, @@ -764,7 +765,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -775,7 +776,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + initialValidatedSettings: map[string]upstreamwatchers.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) @@ -789,7 +790,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -807,7 +808,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}}, + initialValidatedSettings: map[string]upstreamwatchers.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) @@ -821,7 +822,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.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", @@ -831,8 +832,8 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { ldapConnectionValidTrueCondition(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 + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, // newer secret version! + initialValidatedSettings: map[string]upstreamwatchers.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) @@ -846,7 +847,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, }, } @@ -882,7 +883,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { return conn, nil })} - validatedSecretVersionCache := newSecretVersionCache() + validatedSecretVersionCache := upstreamwatchers.NewSecretVersionCache() if tt.initialValidatedSettings != nil { validatedSecretVersionCache.ValidatedSettingsByName = tt.initialValidatedSettings } @@ -936,7 +937,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { // 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{} + tt.wantValidatedSettings = map[string]upstreamwatchers.ValidatedSettings{} } require.Equal(t, tt.wantValidatedSettings, validatedSecretVersionCache.ValidatedSettingsByName) }) diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index 36bd37c8..65b34eea 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go @@ -3,7 +3,22 @@ package upstreamwatchers -import "go.pinniped.dev/internal/constable" +import ( + "context" + "crypto/x509" + "encoding/base64" + "fmt" + "time" + + corev1 "k8s.io/api/core/v1" + corev1informers "k8s.io/client-go/informers/core/v1" + + "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/internal/constable" + "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/upstreamldap" +) const ( ReasonNotFound = "SecretNotFound" @@ -13,4 +28,294 @@ const ( ReasonInvalidTLSConfig = "InvalidTLSConfig" ErrNoCertificates = constable.Error("no certificates found") + + LDAPBindAccountSecretType = corev1.SecretTypeBasicAuth + TestLDAPConnectionTimeout = 90 * time.Second + + // Constants related to conditions. + typeBindSecretValid = "BindSecretValid" + typeTLSConfigurationValid = "TLSConfigurationValid" + typeLDAPConnectionValid = "LDAPConnectionValid" + reasonLDAPConnectionError = "LDAPConnectionError" + noTLSConfigurationMessage = "no TLS configuration provided" + loadedTLSConfigurationMessage = "loaded TLS configuration" ) + +// 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{}} +} + +// read only interface for sharing between ldap and active directory. +type UpstreamGenericLDAPIDP interface { + Spec() UpstreamGenericLDAPSpec + Name() string + Namespace() string + Generation() int64 + Status() UpstreamGenericLDAPStatus +} + +type UpstreamGenericLDAPSpec interface { + Host() string + TLSSpec() *v1alpha1.TLSSpec + BindSecretName() string + UserSearch() UpstreamGenericLDAPUserSearch + GroupSearch() UpstreamGenericLDAPGroupSearch +} + +type UpstreamGenericLDAPUserSearch interface { + Base() string + Filter() string + UsernameAttribute() string + UIDAttribute() string +} + +type UpstreamGenericLDAPGroupSearch interface { + Base() string + Filter() string + GroupNameAttribute() string +} + +type UpstreamGenericLDAPStatus interface { + Conditions() []v1alpha1.Condition +} + +func ValidateTLSConfig(tlsSpec *v1alpha1.TLSSpec, config *upstreamldap.ProviderConfig) *v1alpha1.Condition { + if tlsSpec == nil { + return validTLSCondition(noTLSConfigurationMessage) + } + if len(tlsSpec.CertificateAuthorityData) == 0 { + return validTLSCondition(loadedTLSConfigurationMessage) + } + + bundle, err := base64.StdEncoding.DecodeString(tlsSpec.CertificateAuthorityData) + if err != nil { + return invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", err.Error())) + } + + ca := x509.NewCertPool() + ok := ca.AppendCertsFromPEM(bundle) + if !ok { + return invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", ErrNoCertificates)) + } + + config.CABundle = bundle + return validTLSCondition(loadedTLSConfigurationMessage) +} + +func TestConnection( + ctx context.Context, + bindSecretName string, + 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: typeLDAPConnectionValid, + Status: v1alpha1.ConditionFalse, + Reason: reasonLDAPConnectionError, + 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: typeLDAPConnectionValid, + Status: v1alpha1.ConditionTrue, + Reason: 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, bindSecretName, currentSecretVersion), + } +} + +func HasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(secretVersionCache *SecretVersionCache, currentGeneration int64, upstreamStatusConditions []v1alpha1.Condition, upstreamName string, currentSecretVersion string, config *upstreamldap.ProviderConfig) bool { + for _, cond := range upstreamStatusConditions { + if cond.Type == typeLDAPConnectionValid && 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 := secretVersionCache.ValidatedSettingsByName[upstreamName] + if validatedSecretVersion.BindSecretResourceVersion == currentSecretVersion { + // Reload the TLS vs StartTLS setting that was previously validated. + config.ConnectionProtocol = validatedSecretVersion.LDAPConnectionProtocol + return true + } + } + } + return false +} + +func validTLSCondition(message string) *v1alpha1.Condition { + return &v1alpha1.Condition{ + Type: typeTLSConfigurationValid, + Status: v1alpha1.ConditionTrue, + Reason: ReasonSuccess, + Message: message, + } +} + +func invalidTLSCondition(message string) *v1alpha1.Condition { + return &v1alpha1.Condition{ + Type: typeTLSConfigurationValid, + Status: v1alpha1.ConditionFalse, + Reason: ReasonInvalidTLSConfig, + Message: message, + } +} + +func ValidateSecret(secretInformer corev1informers.SecretInformer, secretName string, secretNamespace string, config *upstreamldap.ProviderConfig) (*v1alpha1.Condition, string) { + secret, err := secretInformer.Lister().Secrets(secretNamespace).Get(secretName) + if err != nil { + return &v1alpha1.Condition{ + Type: typeBindSecretValid, + Status: v1alpha1.ConditionFalse, + Reason: ReasonNotFound, + Message: err.Error(), + }, "" + } + + if secret.Type != corev1.SecretTypeBasicAuth { + return &v1alpha1.Condition{ + Type: typeBindSecretValid, + Status: v1alpha1.ConditionFalse, + Reason: 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: 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: ReasonSuccess, + Message: "loaded bind secret", + }, secret.ResourceVersion +} + +type GradatedConditions struct { + gradatedConditions []GradatedCondition +} + +func (g *GradatedConditions) Conditions() []*v1alpha1.Condition { + conditions := []*v1alpha1.Condition{} + for _, gc := range g.gradatedConditions { + conditions = append(conditions, gc.condition) + } + return conditions +} + +func (g *GradatedConditions) Append(condition *v1alpha1.Condition, isFatal bool) { + g.gradatedConditions = append(g.gradatedConditions, GradatedCondition{condition: condition, isFatal: isFatal}) +} + +// A condition and a boolean that tells you whether it's fatal or just a warning. +type GradatedCondition struct { + condition *v1alpha1.Condition + isFatal bool +} + +func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, secretInformer corev1informers.SecretInformer, validatedSecretVersionsCache *SecretVersionCache, config *upstreamldap.ProviderConfig) GradatedConditions { + conditions := GradatedConditions{} + secretValidCondition, currentSecretVersion := ValidateSecret(secretInformer, upstream.Spec().BindSecretName(), upstream.Namespace(), config) + conditions.Append(secretValidCondition, true) + tlsValidCondition := ValidateTLSConfig(upstream.Spec().TLSSpec(), config) + conditions.Append(tlsValidCondition, true) + + // No point in trying to connect to the server if the config was already determined to be invalid. + var ldapConnectionValidCondition *v1alpha1.Condition + if secretValidCondition.Status == v1alpha1.ConditionTrue && tlsValidCondition.Status == v1alpha1.ConditionTrue { + ldapConnectionValidCondition = validateAndSetLDAPServerConnectivity(ctx, validatedSecretVersionsCache, upstream, config, currentSecretVersion) + if ldapConnectionValidCondition != nil { + conditions.Append(ldapConnectionValidCondition, false) + } + } + return conditions +} + +func validateAndSetLDAPServerConnectivity(ctx context.Context, validatedSecretVersionsCache *SecretVersionCache, upstream UpstreamGenericLDAPIDP, config *upstreamldap.ProviderConfig, currentSecretVersion string) *v1alpha1.Condition { + // TODO refactor validateAndSetLDAPServerConnectivity to be shared and take a helper function for the defaultNamingContext stuff + // so that can be shared. + if HasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(validatedSecretVersionsCache, upstream.Generation(), upstream.Status().Conditions(), upstream.Name(), currentSecretVersion, config) { + return nil + } + + testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, TestLDAPConnectionTimeout) + defer cancelFunc() + + condition := TestConnection(testConnectionTimeout, upstream.Spec().BindSecretName(), 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. + validatedSecretVersionsCache.ValidatedSettingsByName[upstream.Name()] = ValidatedSettings{ + BindSecretResourceVersion: currentSecretVersion, + LDAPConnectionProtocol: config.ConnectionProtocol, + } + } + + return condition +} + +func EvaluateConditions(conditions GradatedConditions, config *upstreamldap.ProviderConfig) (provider.UpstreamLDAPIdentityProviderI, bool) { + for _, gradatedCondition := range conditions.gradatedConditions { + if gradatedCondition.condition.Status != v1alpha1.ConditionTrue && gradatedCondition.isFatal { + // Invalid provider, so do not load it into the cache. + return nil, true + } + } + + for _, gradatedCondition := range conditions.gradatedConditions { + if gradatedCondition.condition.Status != v1alpha1.ConditionTrue && !gradatedCondition.isFatal { + // Error but load it into the cache anyway, treating this condition failure more like a warning. + // Try again hoping that the condition will improve. + return upstreamldap.New(*config), true + } + } + // Fully validated provider, so load it into the cache. + return upstreamldap.New(*config), false +} diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 8b449db8..17c4ec55 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -343,7 +343,7 @@ func requireSuccessfulActiveDirectoryIdentityProviderConditions(t *testing.T, ad require.Equal(t, "loaded bind secret", condition.Message) case "TLSConfigurationValid": require.Equal(t, "loaded TLS configuration", condition.Message) - case "ActiveDirectoryConnectionValid": + case "LDAPConnectionValid": require.Equal(t, expectedActiveDirectoryConnectionValidMessage, condition.Message) } } @@ -351,7 +351,7 @@ func requireSuccessfulActiveDirectoryIdentityProviderConditions(t *testing.T, ad require.ElementsMatch(t, [][]string{ {"BindSecretValid", "True", "Success"}, {"TLSConfigurationValid", "True", "Success"}, - {"ActiveDirectoryConnectionValid", "True", "Success"}, + {"LDAPConnectionValid", "True", "Success"}, }, conditionsSummary) }