Merge pull request #620 from vmware-tanzu/ldap_starttls

Support `StartTLS` for `LDAPIdentityProvider`s

hack/prepare-for-integration-tests.sh

@@ -333,6 +333,7 @@ export PINNIPED_TEST_SUPERVISOR_HTTP_ADDRESS="127.0.0.1:12345"
 export PINNIPED_TEST_SUPERVISOR_HTTPS_ADDRESS="localhost:12344"
 export PINNIPED_TEST_PROXY=http://127.0.0.1:12346
 export PINNIPED_TEST_LDAP_HOST=ldap.tools.svc.cluster.local
+export PINNIPED_TEST_LDAP_STARTTLS_ONLY_HOST=ldapstarttls.tools.svc.cluster.local
 export PINNIPED_TEST_LDAP_LDAPS_CA_BUNDLE="${test_ca_bundle_pem}"
 export PINNIPED_TEST_LDAP_BIND_ACCOUNT_USERNAME="cn=admin,dc=pinniped,dc=dev"
 export PINNIPED_TEST_LDAP_BIND_ACCOUNT_PASSWORD=password

internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go

@@ -26,6 +26,7 @@ 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"
 )
 
@@ -58,13 +59,18 @@ type ldapWatcherController struct {
 }
 
 // An in-memory cache with an entry for each LDAPIdentityProvider, to keep track of which ResourceVersion
-// of the bind Secret was used during the most recent successful validation.
+// of the bind Secret and which TLS/StartTLS setting was used during the most recent successful validation.
 type secretVersionCache struct {
-	ResourceVersionsByName map[string]string
+	ValidatedSettingsByName map[string]validatedSettings
+}
+
+type validatedSettings struct {
+	BindSecretResourceVersion string
+	LDAPConnectionProtocol    upstreamldap.LDAPConnectionProtocol
 }
 
 func newSecretVersionCache() *secretVersionCache {
-	return &secretVersionCache{ResourceVersionsByName: map[string]string{}}
+	return &secretVersionCache{ValidatedSettingsByName: map[string]validatedSettings{}}
 }
 
 // New instantiates a new controllerlib.Controller which will populate the provided UpstreamLDAPIdentityProviderICache.
@@ -228,22 +234,23 @@ func (c *ldapWatcherController) validateTLSConfig(upstream *v1alpha1.LDAPIdentit
 }
 
 func (c *ldapWatcherController) validateFinishedConfig(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider, config *upstreamldap.ProviderConfig, currentSecretVersion string) *v1alpha1.Condition {
-	ldapProvider := upstreamldap.New(*config)
+	if c.hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream, currentSecretVersion, config) {
-
-	if c.hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream, currentSecretVersion) {
 		return nil
 	}
 
 	testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, testLDAPConnectionTimeout)
 	defer cancelFunc()
 
-	condition := c.testConnection(testConnectionTimeout, upstream, config, ldapProvider, currentSecretVersion)
+	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.ResourceVersionsByName[upstream.GetName()] = currentSecretVersion
+		c.validatedSecretVersionsCache.ValidatedSettingsByName[upstream.GetName()] = validatedSettings{
+			BindSecretResourceVersion: currentSecretVersion,
+			LDAPConnectionProtocol:    config.ConnectionProtocol,
+		}
 	}
 
 	return condition
@@ -253,10 +260,31 @@ func (c *ldapWatcherController) testConnection(
 	ctx context.Context,
 	upstream *v1alpha1.LDAPIdentityProvider,
 	config *upstreamldap.ProviderConfig,
-	ldapProvider *upstreamldap.Provider,
 	currentSecretVersion string,
 ) *v1alpha1.Condition {
-	err := ldapProvider.TestConnection(ctx)
+	// 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,
@@ -276,14 +304,16 @@ func (c *ldapWatcherController) testConnection(
 	}
 }
 
-func (c *ldapWatcherController) hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream *v1alpha1.LDAPIdentityProvider, currentSecretVersion string) bool {
+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.ResourceVersionsByName[upstream.GetName()]
+			validatedSecretVersion := c.validatedSecretVersionsCache.ValidatedSettingsByName[upstream.GetName()]
-			if validatedSecretVersion == currentSecretVersion {
+			if validatedSecretVersion.BindSecretResourceVersion == currentSecretVersion {
+				// Reload the TLS vs StartTLS setting that was previously validated.
+				config.ConnectionProtocol = validatedSecretVersion.LDAPConnectionProtocol
 				return true
 			}
 		}

internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/go-ldap/ldap/v3"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
@@ -196,12 +197,13 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 		return deepCopy
 	}
 
-	providerConfigForValidUpstream := &upstreamldap.ProviderConfig{
+	providerConfigForValidUpstreamWithTLS := &upstreamldap.ProviderConfig{
-		Name:         testName,
+		Name:               testName,
-		Host:         testHost,
+		Host:               testHost,
-		CABundle:     testCABundle,
+		ConnectionProtocol: upstreamldap.TLS,
-		BindUsername: testBindUsername,
+		CABundle:           testCABundle,
-		BindPassword: testBindPassword,
+		BindUsername:       testBindUsername,
+		BindPassword:       testBindPassword,
 		UserSearch: upstreamldap.UserSearchConfig{
 			Base:              testUserSearchBase,
 			Filter:            testUserSearchFilter,
@@ -215,6 +217,11 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 		},
 	}
 
+	// Make a copy with targeted changes.
+	copyOfProviderConfigForValidUpstreamWithTLS := *providerConfigForValidUpstreamWithTLS
+	providerConfigForValidUpstreamWithStartTLS := &copyOfProviderConfigForValidUpstreamWithTLS
+	providerConfigForValidUpstreamWithStartTLS.ConnectionProtocol = upstreamldap.StartTLS
+
 	bindSecretValidTrueCondition := func(gen int64) v1alpha1.Condition {
 		return v1alpha1.Condition{
 			Type:               "BindSecretValid",
@@ -264,16 +271,16 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 	}
 
 	tests := []struct {
-		name                           string
+		name                     string
-		initialValidatedSecretVersions map[string]string
+		initialValidatedSettings map[string]validatedSettings
-		inputUpstreams                 []runtime.Object
+		inputUpstreams           []runtime.Object
-		inputSecrets                   []runtime.Object
+		inputSecrets             []runtime.Object
-		setupMocks                     func(conn *mockldapconn.MockConn)
+		setupMocks               func(conn *mockldapconn.MockConn)
-		dialError                      error
+		dialErrors               map[string]error
-		wantErr                        string
+		wantErr                  string
-		wantResultingCache             []*upstreamldap.ProviderConfig
+		wantResultingCache       []*upstreamldap.ProviderConfig
-		wantResultingUpstreams         []v1alpha1.LDAPIdentityProvider
+		wantResultingUpstreams   []v1alpha1.LDAPIdentityProvider
-		wantValidatedSecretVersions    map[string]string
+		wantValidatedSettings    map[string]validatedSettings
 	}{
 		{
 			name:               "no LDAPIdentityProvider upstreams clears the cache",
@@ -288,7 +295,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
 				conn.EXPECT().Close().Times(1)
 			},
-			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstream},
+			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
 			wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
 				Status: v1alpha1.LDAPIdentityProviderStatus{
@@ -296,7 +303,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSecretVersions: map[string]string{testName: "4242"},
+			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name:               "missing secret",
@@ -442,11 +449,12 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 			},
 			wantResultingCache: []*upstreamldap.ProviderConfig{
 				{
-					Name:         testName,
+					Name:               testName,
-					Host:         testHost,
+					Host:               testHost,
-					CABundle:     nil,
+					ConnectionProtocol: upstreamldap.TLS,
-					BindUsername: testBindUsername,
+					CABundle:           nil,
-					BindPassword: testBindPassword,
+					BindUsername:       testBindUsername,
+					BindPassword:       testBindPassword,
 					UserSearch: upstreamldap.UserSearchConfig{
 						Base:              testUserSearchBase,
 						Filter:            testUserSearchFilter,
@@ -478,7 +486,121 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					},
 				},
 			}},
-			wantValidatedSecretVersions: map[string]string{testName: "4242"},
+			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.LDAPIdentityProvider) {
+				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.LDAPIdentityProvider{{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
+				Status: v1alpha1.LDAPIdentityProviderStatus{
+					Phase: "Ready",
+					Conditions: []v1alpha1.Condition{
+						bindSecretValidTrueCondition(1234),
+						{
+							Type:               "LDAPConnectionValid",
+							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,
+						},
+						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.LDAPIdentityProvider) {
+				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.LDAPIdentityProvider{{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
+				Status: v1alpha1.LDAPIdentityProviderStatus{
+					Phase: "Error",
+					Conditions: []v1alpha1.Condition{
+						bindSecretValidTrueCondition(1234),
+						{
+							Type:               "LDAPConnectionValid",
+							Status:             "False",
+							LastTransitionTime: now,
+							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,
+						},
+						tlsConfigurationValidLoadedTrueCondition(1234),
+					},
+				},
+			}},
 		},
 		{
 			name: "non-nil TLS configuration with empty CertificateAuthorityData is valid",
@@ -493,11 +615,12 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 			},
 			wantResultingCache: []*upstreamldap.ProviderConfig{
 				{
-					Name:         testName,
+					Name:               testName,
-					Host:         testHost,
+					Host:               testHost,
-					CABundle:     nil,
+					ConnectionProtocol: upstreamldap.TLS,
-					BindUsername: testBindUsername,
+					CABundle:           nil,
-					BindPassword: testBindPassword,
+					BindUsername:       testBindUsername,
+					BindPassword:       testBindPassword,
 					UserSearch: upstreamldap.UserSearchConfig{
 						Base:              testUserSearchBase,
 						Filter:            testUserSearchFilter,
@@ -518,7 +641,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSecretVersions: map[string]string{testName: "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",
@@ -534,7 +657,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 				conn.EXPECT().Close().Times(1)
 			},
 			wantErr:            controllerlib.ErrSyntheticRequeue.Error(),
-			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstream},
+			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
 			wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{
 				{
 					ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "other-upstream", Generation: 42},
@@ -561,7 +684,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					},
 				},
 			},
-			wantValidatedSecretVersions: map[string]string{testName: "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)",
@@ -569,11 +692,12 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 			inputSecrets:   []runtime.Object{validBindUserSecret("")},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should perform a test dial and bind.
-				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1).Return(errors.New("some bind error"))
+				// Expect two calls to each of these: once for trying TLS and once for trying StartTLS.
-				conn.EXPECT().Close().Times(1)
+				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{providerConfigForValidUpstream},
+			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
 			wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
 				Status: v1alpha1.LDAPIdentityProviderStatus{
@@ -596,19 +720,19 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 			}},
 		},
 		{
-			name: "when the LDAP server connection was already validated for the current resource generation and secret version, then do not validate it again",
+			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.LDAPIdentityProvider) {
 				upstream.Generation = 1234
 				upstream.Status.Conditions = []v1alpha1.Condition{
 					ldapConnectionValidTrueCondition(1234, "4242"),
 				}
 			})},
-			inputSecrets:                   []runtime.Object{validBindUserSecret("4242")},
+			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSecretVersions: map[string]string{testName: "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{providerConfigForValidUpstream},
+			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
 			wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
 				Status: v1alpha1.LDAPIdentityProviderStatus{
@@ -616,7 +740,30 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSecretVersions: map[string]string{testName: "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.LDAPIdentityProvider) {
+				upstream.Generation = 1234
+				upstream.Status.Conditions = []v1alpha1.Condition{
+					ldapConnectionValidTrueCondition(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.LDAPIdentityProvider{{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
+				Status: v1alpha1.LDAPIdentityProviderStatus{
+					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",
@@ -626,14 +773,14 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					ldapConnectionValidTrueCondition(1233, "4242"), // older spec generation!
 				}
 			})},
-			inputSecrets:                   []runtime.Object{validBindUserSecret("4242")},
+			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSecretVersions: map[string]string{testName: "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{providerConfigForValidUpstream},
+			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
 			wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
 				Status: v1alpha1.LDAPIdentityProviderStatus{
@@ -641,7 +788,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSecretVersions: map[string]string{testName: "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",
@@ -658,14 +805,14 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					},
 				}
 			})},
-			inputSecrets:                   []runtime.Object{validBindUserSecret("4242")},
+			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSecretVersions: map[string]string{testName: "1"},
+			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{providerConfigForValidUpstream},
+			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
 			wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
 				Status: v1alpha1.LDAPIdentityProviderStatus{
@@ -673,7 +820,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSecretVersions: map[string]string{testName: "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",
@@ -683,14 +830,14 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					ldapConnectionValidTrueCondition(1234, "4241"), // same spec generation, old secret version
 				}
 			})},
-			inputSecrets:                   []runtime.Object{validBindUserSecret("4242")}, // newer secret version!
+			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},                                                                         // newer secret version!
-			initialValidatedSecretVersions: map[string]string{testName: "4241"},           // old version was validated
+			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{providerConfigForValidUpstream},
+			wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS},
 			wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
 				Status: v1alpha1.LDAPIdentityProviderStatus{
@@ -698,7 +845,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSecretVersions: map[string]string{testName: "4242"},
+			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 	}
 
@@ -724,16 +871,19 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 				tt.setupMocks(conn)
 			}
 
-			dialer := &comparableDialer{upstreamldap.LDAPDialerFunc(func(ctx context.Context, _ string) (upstreamldap.Conn, error) {
+			dialer := &comparableDialer{upstreamldap.LDAPDialerFunc(func(ctx context.Context, hostAndPort string) (upstreamldap.Conn, error) {
-				if tt.dialError != nil {
+				if tt.dialErrors != nil {
-					return nil, tt.dialError
+					dialErr := tt.dialErrors[hostAndPort]
+					if dialErr != nil {
+						return nil, dialErr
+					}
 				}
 				return conn, nil
 			})}
 
 			validatedSecretVersionCache := newSecretVersionCache()
-			if tt.initialValidatedSecretVersions != nil {
+			if tt.initialValidatedSettings != nil {
-				validatedSecretVersionCache.ResourceVersionsByName = tt.initialValidatedSecretVersions
+				validatedSecretVersionCache.ValidatedSettingsByName = tt.initialValidatedSettings
 			}
 
 			controller := newInternal(
@@ -765,11 +915,11 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 			require.Equal(t, len(tt.wantResultingCache), len(actualIDPList))
 			for i := range actualIDPList {
 				actualIDP := actualIDPList[i].(*upstreamldap.Provider)
-				copyOfExpectedValue := *tt.wantResultingCache[i] // copy before edit to avoid race because these tests are run in parallel
+				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.
-				copyOfExpectedValue.Dialer = dialer
+				copyOfExpectedValueForResultingCache.Dialer = dialer
-				require.Equal(t, copyOfExpectedValue, actualIDP.GetConfig())
+				require.Equal(t, copyOfExpectedValueForResultingCache, actualIDP.GetConfig())
 			}
 
 			actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().LDAPIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{})
@@ -784,10 +934,10 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 			}
 
 			// Check that the controller remembered which version of the secret it most recently validated successfully with.
-			if tt.wantValidatedSecretVersions == nil {
+			if tt.wantValidatedSettings == nil {
-				tt.wantValidatedSecretVersions = map[string]string{}
+				tt.wantValidatedSettings = map[string]validatedSettings{}
 			}
-			require.Equal(t, tt.wantValidatedSecretVersions, validatedSecretVersionCache.ResourceVersionsByName)
+			require.Equal(t, tt.wantValidatedSettings, validatedSecretVersionCache.ValidatedSettingsByName)
 		})
 	}
 }

internal/upstreamldap/upstreamldap.go

@@ -60,6 +60,13 @@ func (f LDAPDialerFunc) Dial(ctx context.Context, hostAndPort string) (Conn, err
 	return f(ctx, hostAndPort)
 }
 
+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.
@@ -71,6 +78,9 @@ type ProviderConfig struct {
 	// 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
 
@@ -137,33 +147,48 @@ func (p *Provider) GetConfig() ProviderConfig {
 }
 
 func (p *Provider) dial(ctx context.Context) (Conn, error) {
-	hostAndPort, err := hostAndPortWithDefaultPort(p.c.Host, ldap.DefaultLdapsPort)
+	tlsHostAndPort, err := hostAndPortWithDefaultPort(p.c.Host, ldap.DefaultLdapsPort)
 	if err != nil {
 		return nil, ldap.NewError(ldap.ErrorNetwork, err)
 	}
-	if p.c.Dialer != nil {
+
-		return p.c.Dialer.Dial(ctx, hostAndPort)
+	startTLSHostAndPort, err := hostAndPortWithDefaultPort(p.c.Host, ldap.DefaultLdapPort)
+	if err != nil {
+		return nil, ldap.NewError(ldap.ErrorNetwork, err)
 	}
-	return p.dialTLS(ctx, hostAndPort)
+
+	// Choose how and where to dial based on TLS vs. StartTLS config option.
+	var dialFunc LDAPDialerFunc
+	var hostAndPort string
+	switch {
+	case p.c.ConnectionProtocol == TLS:
+		dialFunc = p.dialTLS
+		hostAndPort = tlsHostAndPort
+	case p.c.ConnectionProtocol == StartTLS:
+		dialFunc = p.dialStartTLS
+		hostAndPort = startTLSHostAndPort
+	default:
+		return nil, ldap.NewError(ldap.ErrorNetwork, fmt.Errorf("did not specify valid ConnectionProtocol"))
+	}
+
+	// Override the real dialer for testing purposes sometimes.
+	if p.c.Dialer != nil {
+		dialFunc = p.c.Dialer.Dial
+	}
+
+	return dialFunc(ctx, hostAndPort)
 }
 
-// dialTLS is the default implementation of the Dialer, used when Dialer is nil.
+// 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, hostAndPort string) (Conn, error) {
-	var rootCAs *x509.CertPool
+	tlsConfig, err := p.tlsConfig()
-	if p.c.CABundle != nil {
+	if err != nil {
-		rootCAs = x509.NewCertPool()
+		return nil, ldap.NewError(ldap.ErrorNetwork, err)
-		if !rootCAs.AppendCertsFromPEM(p.c.CABundle) {
-			return nil, ldap.NewError(ldap.ErrorNetwork, fmt.Errorf("could not parse CA bundle"))
-		}
 	}
 
-	dialer := &tls.Dialer{Config: &tls.Config{
+	dialer := &tls.Dialer{NetDialer: netDialer(), Config: tlsConfig}
-		MinVersion: tls.VersionTLS12,
-		RootCAs:    rootCAs,
-	}}
-
 	c, err := dialer.DialContext(ctx, "tcp", hostAndPort)
 	if err != nil {
 		return nil, ldap.NewError(ldap.ErrorNetwork, err)
@@ -174,6 +199,52 @@ func (p *Provider) dialTLS(ctx context.Context, hostAndPort string) (Conn, error
 	return conn, nil
 }
 
+// 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, hostAndPort string) (Conn, error) {
+	tlsConfig, err := p.tlsConfig()
+	if err != nil {
+		return nil, ldap.NewError(ldap.ErrorNetwork, err)
+	}
+
+	host, err := hostWithoutPort(hostAndPort)
+	if err != nil {
+		return nil, ldap.NewError(ldap.ErrorNetwork, err)
+	}
+	// Unfortunately, this seems to be required for StartTLS, even though it is not needed for regular TLS.
+	tlsConfig.ServerName = host
+
+	c, err := netDialer().DialContext(ctx, "tcp", hostAndPort)
+	if err != nil {
+		return nil, ldap.NewError(ldap.ErrorNetwork, err)
+	}
+
+	conn := ldap.NewConn(c, false)
+	conn.Start()
+	err = conn.StartTLS(tlsConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	return conn, nil
+}
+
+func netDialer() *net.Dialer {
+	return &net.Dialer{Timeout: time.Minute}
+}
+
+func (p *Provider) tlsConfig() (*tls.Config, error) {
+	var rootCAs *x509.CertPool
+	if p.c.CABundle != nil {
+		rootCAs = x509.NewCertPool()
+		if !rootCAs.AppendCertsFromPEM(p.c.CABundle) {
+			return nil, fmt.Errorf("could not parse CA bundle")
+		}
+	}
+	return &tls.Config{MinVersion: tls.VersionTLS12, RootCAs: rootCAs}, nil
+}
+
 // Adds the default port if hostAndPort did not already include a port.
 func hostAndPortWithDefaultPort(hostAndPort string, defaultPort string) (string, error) {
 	host, port, err := net.SplitHostPort(hostAndPort)
@@ -188,7 +259,7 @@ func hostAndPortWithDefaultPort(hostAndPort string, defaultPort string) (string,
 	switch {
 	case port != "" && strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]"):
 		// don't add extra square brackets to an IPv6 address that already has them
-		return host + ":" + port, nil
+		return fmt.Sprintf("%s:%s", host, port), nil
 	case port != "":
 		return net.JoinHostPort(host, port), nil
 	default:
@@ -196,6 +267,22 @@ func hostAndPortWithDefaultPort(hostAndPort string, defaultPort string) (string,
 	}
 }
 
+// Strip the port from a host or host:port.
+func hostWithoutPort(hostAndPort string) (string, error) {
+	host, _, err := net.SplitHostPort(hostAndPort)
+	if err != nil {
+		if strings.HasSuffix(err.Error(), ": missing port in address") { // sad to need to do this string compare
+			return hostAndPort, nil
+		}
+		return "", err // hostAndPort argument was not parsable
+	}
+	if strings.HasPrefix(hostAndPort, "[") {
+		// it was an IPv6 address, so preserve the square brackets.
+		return fmt.Sprintf("[%s]", host), nil
+	}
+	return host, nil
+}
+
 // A name for this upstream provider.
 func (p *Provider) GetName() string {
 	return p.c.Name

internal/upstreamldap/upstreamldap_test.go

@@ -55,11 +55,12 @@ var (
 func TestEndUserAuthentication(t *testing.T) {
 	providerConfig := func(editFunc func(p *ProviderConfig)) *ProviderConfig {
 		config := &ProviderConfig{
-			Name:         "some-provider-name",
+			Name:               "some-provider-name",
-			Host:         testHost,
+			Host:               testHost,
-			CABundle:     nil, // this field is only used by the production dialer, which is replaced by a mock for this test
+			CABundle:           nil, // this field is only used by the production dialer, which is replaced by a mock for this test
-			BindUsername: testBindUsername,
+			ConnectionProtocol: TLS,
-			BindPassword: testBindPassword,
+			BindUsername:       testBindUsername,
+			BindPassword:       testBindPassword,
 			UserSearch: UserSearchConfig{
 				Base:              testUserSearchBase,
 				Filter:            testUserSearchFilter,
@@ -989,12 +990,13 @@ func TestEndUserAuthentication(t *testing.T) {
 func TestTestConnection(t *testing.T) {
 	providerConfig := func(editFunc func(p *ProviderConfig)) *ProviderConfig {
 		config := &ProviderConfig{
-			Name:         "some-provider-name",
+			Name:               "some-provider-name",
-			Host:         testHost,
+			Host:               testHost,
-			CABundle:     nil, // this field is only used by the production dialer, which is replaced by a mock for this test
+			CABundle:           nil, // this field is only used by the production dialer, which is replaced by a mock for this test
-			BindUsername: testBindUsername,
+			ConnectionProtocol: TLS,
-			BindPassword: testBindPassword,
+			BindUsername:       testBindUsername,
-			UserSearch:   UserSearchConfig{}, // not used by TestConnection
+			BindPassword:       testBindPassword,
+			UserSearch:         UserSearchConfig{}, // not used by TestConnection
 		}
 		if editFunc != nil {
 			editFunc(config)
@@ -1132,27 +1134,55 @@ func TestRealTLSDialing(t *testing.T) {
 	tests := []struct {
 		name      string
 		host      string
+		connProto LDAPConnectionProtocol
 		caBundle  []byte
 		context   context.Context
 		wantError string
 	}{
 		{
-			name:     "happy path",
+			name:      "happy path",
-			host:     testServerHostAndPort,
+			host:      testServerHostAndPort,
-			caBundle: []byte(testServerCABundle),
+			caBundle:  []byte(testServerCABundle),
-			context:  context.Background(),
+			connProto: TLS,
+			context:   context.Background(),
 		},
 		{
-			name:      "invalid CA bundle",
+			name:      "invalid CA bundle with TLS",
 			host:      testServerHostAndPort,
 			caBundle:  []byte("not a ca bundle"),
+			connProto: TLS,
 			context:   context.Background(),
 			wantError: `LDAP Result Code 200 "Network Error": could not parse CA bundle`,
 		},
+		{
+			name:      "invalid CA bundle with StartTLS",
+			host:      testServerHostAndPort,
+			caBundle:  []byte("not a ca bundle"),
+			connProto: StartTLS,
+			context:   context.Background(),
+			wantError: `LDAP Result Code 200 "Network Error": could not parse CA bundle`,
+		},
+		{
+			name:      "invalid host with TLS",
+			host:      "this:is:not:a:valid:hostname",
+			caBundle:  []byte(testServerCABundle),
+			connProto: TLS,
+			context:   context.Background(),
+			wantError: `LDAP Result Code 200 "Network Error": address this:is:not:a:valid:hostname: too many colons in address`,
+		},
+		{
+			name:      "invalid host with StartTLS",
+			host:      "this:is:not:a:valid:hostname",
+			caBundle:  []byte(testServerCABundle),
+			connProto: StartTLS,
+			context:   context.Background(),
+			wantError: `LDAP Result Code 200 "Network Error": address this:is:not:a:valid:hostname: too many colons in address`,
+		},
 		{
 			name:      "missing CA bundle when it is required because the host is not using a trusted CA",
 			host:      testServerHostAndPort,
 			caBundle:  nil,
+			connProto: TLS,
 			context:   context.Background(),
 			wantError: `LDAP Result Code 200 "Network Error": x509: certificate signed by unknown authority`,
 		},
@@ -1161,6 +1191,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,
 			context:   context.Background(),
 			wantError: fmt.Sprintf(`LDAP Result Code 200 "Network Error": dial tcp %s: connect: connection refused`, recentlyClaimedHostAndPort),
 		},
@@ -1168,25 +1199,35 @@ func TestRealTLSDialing(t *testing.T) {
 			name:      "pays attention to the passed context",
 			host:      testServerHostAndPort,
 			caBundle:  []byte(testServerCABundle),
+			connProto: TLS,
 			context:   alreadyCancelledContext,
 			wantError: fmt.Sprintf(`LDAP Result Code 200 "Network Error": dial tcp %s: operation was canceled`, testServerHostAndPort),
 		},
+		{
+			name:      "unsupported connection protocol",
+			host:      testServerHostAndPort,
+			caBundle:  []byte(testServerCABundle),
+			connProto: "bad usage of this type",
+			context:   alreadyCancelledContext,
+			wantError: `LDAP Result Code 200 "Network Error": did not specify valid ConnectionProtocol`,
+		},
 	}
 	for _, test := range tests {
-		test := test
+		tt := test
-		t.Run(test.name, func(t *testing.T) {
+		t.Run(tt.name, func(t *testing.T) {
 			provider := New(ProviderConfig{
-				Host:     test.host,
+				Host:               tt.host,
-				CABundle: test.caBundle,
+				CABundle:           tt.caBundle,
-				Dialer:   nil, // this test is for the default (production) dialer
+				ConnectionProtocol: tt.connProto,
+				Dialer:             nil, // this test is for the default (production) TLS dialer
 			})
-			conn, err := provider.dial(test.context)
+			conn, err := provider.dial(tt.context)
 			if conn != nil {
 				defer conn.Close()
 			}
-			if test.wantError != "" {
+			if tt.wantError != "" {
 				require.Nil(t, conn)
-				require.EqualError(t, err, test.wantError)
+				require.EqualError(t, err, tt.wantError)
 			} else {
 				require.NoError(t, err)
 				require.NotNil(t, conn)
@@ -1231,6 +1272,12 @@ func TestHostAndPortWithDefaultPort(t *testing.T) {
 			defaultPort:     "",
 			wantHostAndPort: "host.example.com",
 		},
+		{
+			name:            "host has port and default port is empty",
+			hostAndPort:     "host.example.com:42",
+			defaultPort:     "",
+			wantHostAndPort: "host.example.com:42",
+		},
 		{
 			name:            "IPv6 host already has port",
 			hostAndPort:     "[::1%lo0]:80",
@@ -1257,15 +1304,63 @@ func TestHostAndPortWithDefaultPort(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		test := test
+		tt := test
-		t.Run(test.name, func(t *testing.T) {
+		t.Run(tt.name, func(t *testing.T) {
-			hostAndPort, err := hostAndPortWithDefaultPort(test.hostAndPort, test.defaultPort)
+			hostAndPort, err := hostAndPortWithDefaultPort(tt.hostAndPort, tt.defaultPort)
-			if test.wantError != "" {
+			if tt.wantError != "" {
-				require.EqualError(t, err, test.wantError)
+				require.EqualError(t, err, tt.wantError)
 			} else {
 				require.NoError(t, err)
 			}
-			require.Equal(t, test.wantHostAndPort, hostAndPort)
+			require.Equal(t, tt.wantHostAndPort, hostAndPort)
+		})
+	}
+}
+
+// Test various cases of host and port parsing.
+func TestHostWithoutPort(t *testing.T) {
+	tests := []struct {
+		name            string
+		hostAndPort     string
+		wantError       string
+		wantHostAndPort string
+	}{
+		{
+			name:            "host already has port",
+			hostAndPort:     "host.example.com:99",
+			wantHostAndPort: "host.example.com",
+		},
+		{
+			name:            "host does not have port",
+			hostAndPort:     "host.example.com",
+			wantHostAndPort: "host.example.com",
+		},
+		{
+			name:            "IPv6 host already has port",
+			hostAndPort:     "[::1%lo0]:80",
+			wantHostAndPort: "[::1%lo0]",
+		},
+		{
+			name:            "IPv6 host does not have port",
+			hostAndPort:     "[::1%lo0]",
+			wantHostAndPort: "[::1%lo0]",
+		},
+		{
+			name:        "host is not valid",
+			hostAndPort: "host.example.com:port1:port2",
+			wantError:   "address host.example.com:port1:port2: too many colons in address",
+		},
+	}
+	for _, test := range tests {
+		tt := test
+		t.Run(tt.name, func(t *testing.T) {
+			hostAndPort, err := hostWithoutPort(tt.hostAndPort)
+			if tt.wantError != "" {
+				require.EqualError(t, err, tt.wantError)
+			} else {
+				require.NoError(t, err)
+			}
+			require.Equal(t, tt.wantHostAndPort, hostAndPort)
 		})
 	}
 }

test/deploy/tools/cert-issuer.yaml

@@ -77,13 +77,15 @@ spec:
             | cfssljson -bare dex
 
           # Cheat and add 127.0.0.1 as an IP SAN so we can use the ldaps port through port forwarding.
+          # Also allow the server to be accessed by multiple Service names to different Services
+          # can provide/hide different ports.
           echo "generating LDAP server certificate..."
           cfssl gencert \
             -ca ca.pem -ca-key ca-key.pem \
             -config /tmp/cfssl-default.json \
             -profile www \
             -cn "ldap.tools.svc.cluster.local" \
-            -hostname "ldap.tools.svc.cluster.local,127.0.0.1" \
+            -hostname "ldap.tools.svc.cluster.local,ldaps.tools.svc.cluster.local,ldapstarttls.tools.svc.cluster.local,127.0.0.1" \
             /tmp/csr.json \
             | cfssljson -bare ldap
 

test/deploy/tools/ldap.yaml

@@ -127,6 +127,63 @@ metadata:
 type: Opaque
 stringData: #@ ldapLIDIF()
 ---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: ldap-server-config-before-ldif-files
+  namespace: tools
+type: Opaque
+stringData:
+  server-config.ldif: |
+    # Load the memberof module.
+    dn: cn=module,cn=config
+    cn: module
+    objectClass: olcModuleList
+    objectClass: top
+    olcModulePath: /opt/bitnami/openldap/lib/openldap
+    olcModuleLoad: memberof
+
+    dn: olcOverlay={0}memberof,olcDatabase={2}hdb,cn=config
+    objectClass: olcConfig
+    objectClass: olcMemberOf
+    objectClass: olcOverlayConfig
+    objectClass: top
+    olcOverlay: memberof
+    olcMemberOfDangling: ignore
+    olcMemberOfRefInt: TRUE
+    olcMemberOfGroupOC: groupOfNames
+    olcMemberOfMemberAD: member
+
+    # Load the refint module.
+    dn: cn=module,cn=config
+    cn: module
+    objectclass: olcModuleList
+    objectclass: top
+    olcmodulepath: /opt/bitnami/openldap/lib/openldap
+    olcmoduleload: refint
+
+    dn: olcOverlay={1}refint,olcDatabase={2}hdb,cn=config
+    objectClass: olcConfig
+    objectClass: olcOverlayConfig
+    objectClass: olcRefintConfig
+    objectClass: top
+    olcOverlay: {1}refint
+    olcRefintAttribute: memberof member manager owner
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: ldap-server-config-after-ldif-files
+  namespace: tools
+type: Opaque
+stringData:
+  server-config.ldif: |
+    # Reject any further connections that do not use TLS or StartTLS
+    dn: olcDatabase={2}hdb,cn=config
+    changetype: modify
+    add: olcSecurity
+    olcSecurity: tls=1
+---
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -149,7 +206,10 @@ spec:
     spec:
       containers:
         - name: ldap
-          image: docker.io/bitnami/openldap
+          #! Use our own fork of docker.io/bitnami/openldap for now, because we added the
+          #! LDAP_SERVER_CONFIG_BEFORE_CUSTOM_LDIF_DIR and LDAP_SERVER_CONFIG_AFTER_CUSTOM_LDIF_DIR options.
+          #! See https://github.com/pinniped-ci-bot/bitnami-docker-openldap/tree/pinniped
+          image: projects.registry.vmware.com/pinniped/test-ldap:latest
           imagePullPolicy: Always
           ports:
             - name: ldap
@@ -173,8 +233,14 @@ spec:
           env:
             #! Example ldapsearch commands that can be run from within the container based on these env vars.
             #! These will print the whole LDAP tree starting at our root.
-            #!  ldapsearch -x -H 'ldap://ldap.tools.svc.cluster.local' -D 'cn=admin,dc=pinniped,dc=dev' -w password -b 'dc=pinniped,dc=dev'
+            #! Using StartTLS (-ZZ) on the ldap port...
-            #!  LDAPTLS_CACERT=/var/certs/ca.pem ldapsearch -x -H 'ldaps://ldap.tools.svc.cluster.local' -D 'cn=admin,dc=pinniped,dc=dev' -w password -b 'dc=pinniped,dc=dev'
+            #!   LDAPTLS_CACERT=/var/certs/ca.pem ldapsearch -x -ZZ -H 'ldap://ldap.tools.svc.cluster.local' -D 'cn=admin,dc=pinniped,dc=dev' -w password -b 'dc=pinniped,dc=dev'
+            #! Using ldaps...
+            #!   LDAPTLS_CACERT=/var/certs/ca.pem ldapsearch -x -H 'ldaps://ldap.tools.svc.cluster.local' -D 'cn=admin,dc=pinniped,dc=dev' -w password -b 'dc=pinniped,dc=dev'
+            #! Note that the memberOf attribute is special and not returned by default. It must be specified as one of attributes to return in the search, e.g.:
+            #!   LDAPTLS_CACERT=/var/certs/ca.pem ldapsearch -x -H 'ldaps://ldap.tools.svc.cluster.local' -D 'cn=admin,dc=pinniped,dc=dev' -w password -b 'dc=pinniped,dc=dev' cn uidNumber mail member memberOf
+            #! This should fail and report "TLS confidentiality required" because we require TLS and this does not use TLS or StartTLS...
+            #!   ldapsearch -x -H 'ldap://ldap.tools.svc.cluster.local' -D 'cn=admin,dc=pinniped,dc=dev' -w password -b 'dc=pinniped,dc=dev'
             - name: BITNAMI_DEBUG
               value: "true"
             - name: LDAP_ADMIN_USERNAME
@@ -192,6 +258,10 @@ spec:
               #! Note that the custom LDIF file is only read at pod start-up time.
             - name: LDAP_CUSTOM_LDIF_DIR
               value: "/var/ldifs"
+            - name: LDAP_SERVER_CONFIG_BEFORE_CUSTOM_LDIF_DIR
+              value: "/var/server-config-before-ldifs"
+            - name: LDAP_SERVER_CONFIG_AFTER_CUSTOM_LDIF_DIR
+              value: "/var/server-config-after-ldifs"
               #! Seems like LDAP_ROOT is still required when using LDAP_CUSTOM_LDIF_DIR because it effects the admin user.
               #! Presumably this needs to match the root that we create in the LDIF file.
             - name: LDAP_ROOT
@@ -203,6 +273,12 @@ spec:
             - name: ldifs
               mountPath: /var/ldifs
               readOnly: true
+            - name: server-config-before-ldifs
+              mountPath: /var/server-config-before-ldifs
+              readOnly: true
+            - name: server-config-after-ldifs
+              mountPath: /var/server-config-after-ldifs
+              readOnly: true
       volumes:
         - name: certs
           secret:
@@ -210,6 +286,12 @@ spec:
         - name: ldifs
           secret:
             secretName: ldap-ldif-files
+        - name: server-config-before-ldifs
+          secret:
+            secretName: ldap-server-config-before-ldif-files
+        - name: server-config-after-ldifs
+          secret:
+            secretName: ldap-server-config-after-ldif-files
 ---
 apiVersion: v1
 kind: Service
@@ -231,3 +313,37 @@ spec:
       port: 636
       targetPort: 1636
       name: ldaps
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: ldaps
+  namespace: tools
+  labels:
+    app: ldap
+spec:
+  type: ClusterIP
+  selector:
+    app: ldap
+  ports:
+    - protocol: TCP
+      port: 636
+      targetPort: 1636
+      name: ldaps
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: ldapstarttls
+  namespace: tools
+  labels:
+    app: ldap
+spec:
+  type: ClusterIP
+  selector:
+    app: ldap
+  ports:
+    - protocol: TCP
+      port: 389
+      targetPort: 1389
+      name: ldap

test/integration/ldap_client_test.go

@@ -37,15 +37,19 @@ func TestLDAPSearch(t *testing.T) {
 		cancelFunc() // this will send SIGKILL to the subprocess, just in case
 	})
 
-	hostPorts := findRecentlyUnusedLocalhostPorts(t, 2)
+	localhostPorts := findRecentlyUnusedLocalhostPorts(t, 3)
-	ldapHostPort := hostPorts[0]
+	ldapLocalhostPort := localhostPorts[0]
-	unusedHostPort := hostPorts[1]
+	ldapsLocalhostPort := localhostPorts[1]
+	unusedLocalhostPort := localhostPorts[2]
 
 	// Expose the the test LDAP server's TLS port on the localhost.
-	startKubectlPortForward(ctx, t, ldapHostPort, "ldaps", "ldap", env.ToolsNamespace)
+	startKubectlPortForward(ctx, t, ldapsLocalhostPort, "ldaps", "ldap", env.ToolsNamespace)
+
+	// Expose the the test LDAP server's StartTLS port on the localhost.
+	startKubectlPortForward(ctx, t, ldapLocalhostPort, "ldap", "ldap", env.ToolsNamespace)
 
 	providerConfig := func(editFunc func(p *upstreamldap.ProviderConfig)) *upstreamldap.ProviderConfig {
-		providerConfig := defaultProviderConfig(env, ldapHostPort)
+		providerConfig := defaultProviderConfig(env, ldapsLocalhostPort)
 		if editFunc != nil {
 			editFunc(providerConfig)
 		}
@@ -64,7 +68,7 @@ func TestLDAPSearch(t *testing.T) {
 		wantUnauthenticated bool
 	}{
 		{
-			name:     "happy path",
+			name:     "happy path with TLS",
 			username: "pinny",
 			password: pinnyPassword,
 			provider: upstreamldap.New(*providerConfig(nil)),
@@ -72,6 +76,18 @@ func TestLDAPSearch(t *testing.T) {
 				User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}},
 			},
 		},
+		{
+			name:     "happy path with StartTLS",
+			username: "pinny",
+			password: pinnyPassword,
+			provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) {
+				p.Host = "127.0.0.1:" + ldapLocalhostPort
+				p.ConnectionProtocol = upstreamldap.StartTLS
+			})),
+			wantAuthResponse: &authenticator.Response{
+				User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}},
+			},
+		},
 		{
 			name:     "using a different user search base",
 			username: "pinny",
@@ -251,6 +267,17 @@ func TestLDAPSearch(t *testing.T) {
 			provider:  upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.BindPassword = "wrong-password" })),
 			wantError: `error binding as "cn=admin,dc=pinniped,dc=dev" before user search: LDAP Result Code 49 "Invalid Credentials": `,
 		},
+		{
+			name:     "when the bind user username is wrong with StartTLS: example of an error after successful connection with StartTLS",
+			username: "pinny",
+			password: pinnyPassword,
+			provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) {
+				p.Host = "127.0.0.1:" + ldapLocalhostPort
+				p.ConnectionProtocol = upstreamldap.StartTLS
+				p.BindUsername = "cn=wrong,dc=pinniped,dc=dev"
+			})),
+			wantError: `error binding as "cn=wrong,dc=pinniped,dc=dev" before user search: LDAP Result Code 49 "Invalid Credentials": `,
+		},
 		{
 			name:                "when the end user password is wrong",
 			username:            "pinny",
@@ -296,32 +323,89 @@ func TestLDAPSearch(t *testing.T) {
 			wantError: `error searching for user "pinny": LDAP Result Code 4 "Size Limit Exceeded": `,
 		},
 		{
-			name:      "when the server is unreachable",
+			name:      "when the server is unreachable with TLS",
 			username:  "pinny",
 			password:  pinnyPassword,
-			provider:  upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.Host = "127.0.0.1:" + unusedHostPort })),
+			provider:  upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.Host = "127.0.0.1:" + unusedLocalhostPort })),
-			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": dial tcp 127.0.0.1:%s: connect: connection refused`, unusedHostPort, unusedHostPort),
+			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": dial tcp 127.0.0.1:%s: connect: connection refused`, unusedLocalhostPort, unusedLocalhostPort),
 		},
 		{
-			name:      "when the server is not parsable",
+			name:     "when the server is unreachable with StartTLS",
+			username: "pinny",
+			password: pinnyPassword,
+			provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) {
+				p.Host = "127.0.0.1:" + unusedLocalhostPort
+				p.ConnectionProtocol = upstreamldap.StartTLS
+			})),
+			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": dial tcp 127.0.0.1:%s: connect: connection refused`, unusedLocalhostPort, unusedLocalhostPort),
+		},
+		{
+			name:      "when the server is not parsable with TLS",
 			username:  "pinny",
 			password:  pinnyPassword,
 			provider:  upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.Host = "too:many:ports" })),
 			wantError: `error dialing host "too:many:ports": LDAP Result Code 200 "Network Error": address too:many:ports: too many colons in address`,
 		},
 		{
-			name:      "when the CA bundle is not parsable",
+			name:     "when the server is not parsable with StartTLS",
+			username: "pinny",
+			password: pinnyPassword,
+			provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) {
+				p.Host = "127.0.0.1:" + ldapLocalhostPort
+				p.ConnectionProtocol = upstreamldap.StartTLS
+				p.Host = "too:many:ports"
+			})),
+			wantError: `error dialing host "too:many:ports": LDAP Result Code 200 "Network Error": address too:many:ports: too many colons in address`,
+		},
+		{
+			name:      "when the CA bundle is not parsable with TLS",
 			username:  "pinny",
 			password:  pinnyPassword,
 			provider:  upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.CABundle = []byte("invalid-pem") })),
-			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": could not parse CA bundle`, ldapHostPort),
+			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": could not parse CA bundle`, ldapsLocalhostPort),
 		},
 		{
-			name:      "when the CA bundle does not cause the host to be trusted",
+			name:     "when the CA bundle is not parsable with StartTLS",
+			username: "pinny",
+			password: pinnyPassword,
+			provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) {
+				p.Host = "127.0.0.1:" + ldapLocalhostPort
+				p.ConnectionProtocol = upstreamldap.StartTLS
+				p.CABundle = []byte("invalid-pem")
+			})),
+			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": could not parse CA bundle`, ldapLocalhostPort),
+		},
+		{
+			name:      "when the CA bundle does not cause the host to be trusted with TLS",
 			username:  "pinny",
 			password:  pinnyPassword,
 			provider:  upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.CABundle = nil })),
-			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": x509: certificate signed by unknown authority`, ldapHostPort),
+			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": x509: certificate signed by unknown authority`, ldapsLocalhostPort),
+		},
+		{
+			name:     "when the CA bundle does not cause the host to be trusted with StartTLS",
+			username: "pinny",
+			password: pinnyPassword,
+			provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) {
+				p.Host = "127.0.0.1:" + ldapLocalhostPort
+				p.ConnectionProtocol = upstreamldap.StartTLS
+				p.CABundle = nil
+			})),
+			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": TLS handshake failed (x509: certificate signed by unknown authority)`, ldapLocalhostPort),
+		},
+		{
+			name:      "when trying to use TLS to connect to a port which only supports StartTLS",
+			username:  "pinny",
+			password:  pinnyPassword,
+			provider:  upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.Host = "127.0.0.1:" + ldapLocalhostPort })),
+			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": LDAP Result Code 200 "Network Error": EOF`, ldapLocalhostPort),
+		},
+		{
+			name:      "when trying to use StartTLS to connect to a port which only supports TLS",
+			username:  "pinny",
+			password:  pinnyPassword,
+			provider:  upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.ConnectionProtocol = upstreamldap.StartTLS })),
+			wantError: fmt.Sprintf(`error dialing host "127.0.0.1:%s": unable to read LDAP response packet: unexpected EOF`, ldapsLocalhostPort),
 		},
 		{
 			name:      "when the UsernameAttribute attribute has multiple values in the entry",
@@ -541,13 +625,14 @@ type authUserResult struct {
 	err           error
 }
 
-func defaultProviderConfig(env *library.TestEnv, ldapHostPort string) *upstreamldap.ProviderConfig {
+func defaultProviderConfig(env *library.TestEnv, port string) *upstreamldap.ProviderConfig {
 	return &upstreamldap.ProviderConfig{
-		Name:         "test-ldap-provider",
+		Name:               "test-ldap-provider",
-		Host:         "127.0.0.1:" + ldapHostPort,
+		Host:               "127.0.0.1:" + port,
-		CABundle:     []byte(env.SupervisorUpstreamLDAP.CABundle),
+		ConnectionProtocol: upstreamldap.TLS,
-		BindUsername: "cn=admin,dc=pinniped,dc=dev",
+		CABundle:           []byte(env.SupervisorUpstreamLDAP.CABundle),
-		BindPassword: "password",
+		BindUsername:       "cn=admin,dc=pinniped,dc=dev",
+		BindPassword:       "password",
 		UserSearch: upstreamldap.UserSearchConfig{
 			Base:              "ou=users,dc=pinniped,dc=dev",
 			Filter:            "", // defaults to UsernameAttribute={}, i.e. "cn={}" in this case

test/integration/supervisor_login_test.go

@@ -69,7 +69,7 @@ func TestSupervisorLogin(t *testing.T) {
 			wantDownstreamIDTokenGroups:          env.SupervisorUpstreamOIDC.ExpectedGroups,
 		},
 		{
-			name: "ldap with email as username and groups names as DNs",
+			name: "ldap with email as username and groups names as DNs and using an LDAP provider which supports TLS",
 			createIDP: func(t *testing.T) {
 				t.Helper()
 				secret := library.CreateTestSecret(t, env.SupervisorNamespace, "ldap-service-account", v1.SecretTypeBasicAuth,
@@ -126,7 +126,7 @@ func TestSupervisorLogin(t *testing.T) {
 			wantDownstreamIDTokenGroups:          env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs,
 		},
 		{
-			name: "ldap with CN as username and group names as CNs", // try another variation of configuration options
+			name: "ldap with CN as username and group names as CNs and using an LDAP provider which only supports StartTLS", // try another variation of configuration options
 			createIDP: func(t *testing.T) {
 				t.Helper()
 				secret := library.CreateTestSecret(t, env.SupervisorNamespace, "ldap-service-account", v1.SecretTypeBasicAuth,
@@ -136,7 +136,7 @@ func TestSupervisorLogin(t *testing.T) {
 					},
 				)
 				ldapIDP := library.CreateTestLDAPIdentityProvider(t, idpv1alpha1.LDAPIdentityProviderSpec{
-					Host: env.SupervisorUpstreamLDAP.Host,
+					Host: env.SupervisorUpstreamLDAP.StartTLSOnlyHost,
 					TLS: &idpv1alpha1.TLSSpec{
 						CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.CABundle)),
 					},
@@ -161,7 +161,7 @@ func TestSupervisorLogin(t *testing.T) {
 				}, idpv1alpha1.LDAPPhaseReady)
 				expectedMsg := fmt.Sprintf(
 					`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
-					env.SupervisorUpstreamLDAP.Host, env.SupervisorUpstreamLDAP.BindUsername,
+					env.SupervisorUpstreamLDAP.StartTLSOnlyHost, env.SupervisorUpstreamLDAP.BindUsername,
 					secret.Name, secret.ResourceVersion,
 				)
 				requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg)
@@ -176,7 +176,7 @@ func TestSupervisorLogin(t *testing.T) {
 			},
 			// the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute
 			wantDownstreamIDTokenSubjectToMatch: regexp.QuoteMeta(
-				"ldaps://" + env.SupervisorUpstreamLDAP.Host + "?sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue,
+				"ldaps://" + env.SupervisorUpstreamLDAP.StartTLSOnlyHost + "?sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue,
 			),
 			// the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute
 			wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserDN),

test/library/client.go

@@ -437,7 +437,10 @@ func CreateTestLDAPIdentityProvider(t *testing.T, spec idpv1alpha1.LDAPIdentityP
 			return false
 		}
 		return result.Status.Phase == expectedPhase
-	}, 60*time.Second, 1*time.Second, "expected the LDAPIdentityProvider to go into phase %s, LDAPIdentityProvider was: %s", expectedPhase, Sdump(result))
+	},
+		2*time.Minute, // it takes 1 minute for a failed LDAP TLS connection test to timeout before it tries using StartTLS, so wait longer than that
+		1*time.Second,
+		"expected the LDAPIdentityProvider to go into phase %s, LDAPIdentityProvider was: %s", expectedPhase, Sdump(result))
 	return result
 }
 

test/library/env.go

@@ -73,6 +73,7 @@ type TestOIDCUpstream struct {
 
 type TestLDAPUpstream struct {
 	Host                           string   `json:"host"`
+	StartTLSOnlyHost               string   `json:"startTLSOnlyHost"`
 	CABundle                       string   `json:"caBundle"`
 	BindUsername                   string   `json:"bindUsername"`
 	BindPassword                   string   `json:"bindPassword"`
@@ -240,6 +241,7 @@ func loadEnvVars(t *testing.T, result *TestEnv) {
 
 	result.SupervisorUpstreamLDAP = TestLDAPUpstream{
 		Host:                           needEnv(t, "PINNIPED_TEST_LDAP_HOST"),
+		StartTLSOnlyHost:               needEnv(t, "PINNIPED_TEST_LDAP_STARTTLS_ONLY_HOST"),
 		CABundle:                       base64Decoded(t, os.Getenv("PINNIPED_TEST_LDAP_LDAPS_CA_BUNDLE")),
 		BindUsername:                   needEnv(t, "PINNIPED_TEST_LDAP_BIND_ACCOUNT_USERNAME"),
 		BindPassword:                   needEnv(t, "PINNIPED_TEST_LDAP_BIND_ACCOUNT_PASSWORD"),