From 8b549f66d4623b6095050dc64c1a1b07c3946a4a Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 20 May 2021 13:39:48 -0700 Subject: [PATCH] Add integration test for LDAP StartTLS --- hack/prepare-for-integration-tests.sh | 1 + .../ldapupstreamwatcher/ldap_upstream_watcher.go | 4 ++++ test/integration/supervisor_login_test.go | 10 +++++----- test/library/client.go | 5 ++++- test/library/env.go | 2 ++ 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 0313b60a..b918049b 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/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 diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index 180b9ef4..3f1707b9 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/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" ) @@ -266,15 +267,18 @@ func (c *ldapWatcherController) testConnection( 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 diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 41d97aeb..1690c109 100644 --- a/test/integration/supervisor_login_test.go +++ b/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), diff --git a/test/library/client.go b/test/library/client.go index f256444e..c9e9baca 100644 --- a/test/library/client.go +++ b/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 } diff --git a/test/library/env.go b/test/library/env.go index e4103843..d8e7b44c 100644 --- a/test/library/env.go +++ b/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"),