diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 2c21d99d..d6490846 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -207,7 +207,7 @@ type UpstreamActiveDirectoryIdentityProviderICache interface { type activeDirectoryWatcherController struct { cache UpstreamActiveDirectoryIdentityProviderICache - validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache + validatedSecretVersionsCache upstreamwatchers.SecretVersionCacheI ldapDialer upstreamldap.LDAPDialer client pinnipedclientset.Interface activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer @@ -238,7 +238,7 @@ func New( // For test dependency injection purposes. func newInternal( idpCache UpstreamActiveDirectoryIdentityProviderICache, - validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache, + validatedSecretVersionsCache upstreamwatchers.SecretVersionCacheI, ldapDialer upstreamldap.LDAPDialer, client pinnipedclientset.Interface, activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer, 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 a2326ac2..33aea395 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -370,7 +370,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { name: "missing secret", @@ -555,7 +555,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { name: "sAMAccountName explicitly provided as group name attribute does not add an override", @@ -610,7 +610,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { name: "when TLS connection fails it tries to use StartTLS instead: without a specified port it automatically switches ports", @@ -670,7 +670,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { name: "when TLS connection fails it tries to use StartTLS instead: with a specified port it does not automatically switch ports", @@ -729,7 +729,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { name: "non-nil TLS configuration with empty CertificateAuthorityData is valid", @@ -771,7 +771,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { name: "one valid upstream and one invalid upstream updates the cache to include only the valid upstream", @@ -814,10 +814,12 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { - name: "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway (treated like a warning)", + name: "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway but not to validatedsettings (treated like a warning)", + // If we can't connect, we can still try to allow users to log in, but update the conditions to say that there's a problem + // Also don't add anything to the validated settings so that the next time this runs we can try again. inputUpstreams: []runtime.Object{validUpstream}, inputSecrets: []runtime.Object{validBindUserSecret("")}, setupMocks: func(conn *mockldapconn.MockConn) { @@ -849,10 +851,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { name: "when testing the connection to the LDAP server fails, but later querying defaultsearchbase succeeds, then the upstream is still added to the cache anyway (treated like a warning)", + // Add to cache but not to validatedSettings so we recheck next time inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { upstream.Spec.UserSearch.Base = "" })}, @@ -909,7 +912,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { name: "when testing the connection to the LDAP server fails, and querying defaultsearchbase fails, then the upstream is not added to the cache", @@ -945,7 +948,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { 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", @@ -953,10 +956,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { upstream.Generation = 1234 upstream.Status.Conditions = []v1alpha1.Condition{ activeDirectoryConnectionValidTrueCondition(1234, "4242"), + searchBaseFoundInConfigCondition(1234), } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, 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. }, @@ -968,10 +972,12 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Conditions: allConditionsTrue(1234, "4242"), }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { - name: "when the LDAP server connection was already validated using TLS, but the search base wasn't, load TLS into the config and try again for the search base", + name: "when the validated cache contains LDAP server info but the search base is empty, reload everything", + // this is an invalid state that shouldn't happen now, but if it does we should consider the whole + // validatedsettings cache invalid. inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { upstream.Generation = 1234 upstream.Status.Conditions = []v1alpha1.Condition{ @@ -980,10 +986,10 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { upstream.Spec.UserSearch.Base = "" })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, Generation: 1234}}, setupMocks: func(conn *mockldapconn.MockConn) { - conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) - conn.EXPECT().Close().Times(1) + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(2) + conn.EXPECT().Close().Times(2) conn.EXPECT().Search(expectedDefaultNamingContextSearch()).Return(exampleDefaultNamingContextSearchResult, nil).Times(1) }, wantResultingCache: []*upstreamldap.ProviderConfig{ @@ -1020,7 +1026,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { name: "when the LDAP server connection was already validated using TLS, and the search base was found, load TLS and search base info into the cache", @@ -1033,7 +1039,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { upstream.Spec.UserSearch.Base = "" })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase}}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, setupMocks: func(conn *mockldapconn.MockConn) { }, wantResultingCache: []*upstreamldap.ProviderConfig{ @@ -1075,6 +1081,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, }, { @@ -1083,10 +1090,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { upstream.Generation = 1234 upstream.Status.Conditions = []v1alpha1.Condition{ activeDirectoryConnectionValidTrueCondition(1234, "4242"), + searchBaseFoundInConfigCondition(1234), } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS, Generation: 1234, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase}}, 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. }, @@ -1103,6 +1111,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, }, { @@ -1119,6 +1128,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1233, }}, setupMocks: func(conn *mockldapconn.MockConn) { // Should perform a test dial and bind. @@ -1138,6 +1148,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, }, { @@ -1156,7 +1167,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { } })}, inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS, Generation: 1234}}, setupMocks: func(conn *mockldapconn.MockConn) { // Should perform a test dial and bind. conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) @@ -1175,6 +1186,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, }, { @@ -1191,6 +1203,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, // old version was validated setupMocks: func(conn *mockldapconn.MockConn) { // Should perform a test dial and bind. @@ -1210,6 +1223,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, }, { @@ -1261,6 +1275,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, }, { @@ -1311,7 +1326,13 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: exampleDefaultNamingContext}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: { + BindSecretResourceVersion: "4242", + LDAPConnectionProtocol: upstreamldap.TLS, + UserSearchBase: exampleDefaultNamingContext, + GroupSearchBase: exampleDefaultNamingContext, + Generation: 1234, + }}, }, { name: "when the input activedirectoryidentityprovider leaves user search base blank but provides group search base, query for defaultNamingContext", @@ -1360,7 +1381,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, Generation: 1234}}, }, { name: "when the input activedirectoryidentityprovider leaves group search base blank but provides user search base, query for defaultNamingContext", @@ -1409,7 +1430,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: exampleDefaultNamingContext}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: exampleDefaultNamingContext, Generation: 1234}}, }, { name: "when the input activedirectoryidentityprovider leaves group search base blank and query for defaultNamingContext fails", @@ -1437,10 +1458,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{ - testName: {BindSecretResourceVersion: "4242", - LDAPConnectionProtocol: upstreamldap.TLS, - UserSearchBase: testUserSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { name: "when query for defaultNamingContext returns empty string", @@ -1476,10 +1494,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{ - testName: {BindSecretResourceVersion: "4242", - LDAPConnectionProtocol: upstreamldap.TLS, - UserSearchBase: testUserSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { name: "when query for defaultNamingContext returns multiple entries", @@ -1521,10 +1536,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{ - testName: {BindSecretResourceVersion: "4242", - LDAPConnectionProtocol: upstreamldap.TLS, - UserSearchBase: testUserSearchBase}}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { name: "when query for defaultNamingContext returns no entries", @@ -1553,10 +1565,73 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, + }, + { + name: "when search base was previously found but the bind secret has changed", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Generation = 1234 + upstream.Status.Conditions = []v1alpha1.Condition{ + searchBaseFoundInRootDSECondition(1234), + } + upstream.Spec.UserSearch.Attributes = v1alpha1.ActiveDirectoryIdentityProviderUserSearchAttributes{} + upstream.Spec.GroupSearch.Base = "" + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: { + BindSecretResourceVersion: "4241", + LDAPConnectionProtocol: upstreamldap.TLS, + UserSearchBase: testUserSearchBase, + GroupSearchBase: testGroupSearchBase, + Generation: 1234, + }}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a test dial and bind. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(2) + conn.EXPECT().Close().Times(2) + conn.EXPECT().Search(expectedDefaultNamingContextSearch()).Return(exampleDefaultNamingContextSearchResult, nil).Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{ + { + Name: testName, + Host: testHost, + ConnectionProtocol: upstreamldap.TLS, + CABundle: testCABundle, + BindUsername: testBindUsername, + BindPassword: testBindPassword, + UserSearch: upstreamldap.UserSearchConfig{ + Base: testUserSearchBase, + Filter: testUserSearchFilter, + UsernameAttribute: "userPrincipalName", + UIDAttribute: "objectGUID", + }, + GroupSearch: upstreamldap.GroupSearchConfig{ + Base: exampleDefaultNamingContext, + Filter: testGroupSearchFilter, + GroupNameAttribute: testGroupNameAttrName, + }, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}, + }, + }, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), + activeDirectoryConnectionValidTrueCondition(1234, "4242"), + searchBaseFoundInRootDSECondition(1234), + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{ testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, - UserSearchBase: testUserSearchBase}}, + GroupSearchBase: exampleDefaultNamingContext, + UserSearchBase: testUserSearchBase, + Generation: 1234, + }}, }, } @@ -1592,9 +1667,15 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { return conn, nil })} - validatedSecretVersionCache := upstreamwatchers.NewSecretVersionCache() + var validatedSecretVersionCache *upstreamwatchers.SecretVersionCache if tt.initialValidatedSettings != nil { - validatedSecretVersionCache.ValidatedSettingsByName = tt.initialValidatedSettings + validatedSecretVersionCache = &upstreamwatchers.SecretVersionCache{ + ValidatedSettingsByName: tt.initialValidatedSettings, + } + } else { + validatedSecretVersionCache = &upstreamwatchers.SecretVersionCache{ + ValidatedSettingsByName: map[string]upstreamwatchers.ValidatedSettings{}, + } } controller := newInternal( diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index 90d172b1..af748da1 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -134,7 +134,7 @@ type UpstreamLDAPIdentityProviderICache interface { type ldapWatcherController struct { cache UpstreamLDAPIdentityProviderICache - validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache + validatedSecretVersionsCache upstreamwatchers.SecretVersionCacheI ldapDialer upstreamldap.LDAPDialer client pinnipedclientset.Interface ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer @@ -165,7 +165,7 @@ func New( // For test dependency injection purposes. func newInternal( idpCache UpstreamLDAPIdentityProviderICache, - validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache, + validatedSecretVersionsCache upstreamwatchers.SecretVersionCacheI, ldapDialer upstreamldap.LDAPDialer, client pinnipedclientset.Interface, ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer, diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index 16993640..e9ab4813 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -310,6 +310,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, }, { @@ -498,6 +499,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}}, { name: "when TLS connection fails it tries to use StartTLS instead: without a specified port it automatically switches ports", @@ -560,6 +562,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}}, { name: "when TLS connection fails it tries to use StartTLS instead: with a specified port it does not automatically switch ports", @@ -616,10 +619,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: { - UserSearchBase: testUserSearchBase, - GroupSearchBase: testGroupSearchBase, - }}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { name: "non-nil TLS configuration with empty CertificateAuthorityData is valid", @@ -665,6 +665,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}, }, { @@ -713,9 +714,10 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}}, { - name: "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway (treated like a warning)", + name: "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway (treated like a warning) but not the validated settings cache", inputUpstreams: []runtime.Object{validUpstream}, inputSecrets: []runtime.Object{validBindUserSecret("")}, setupMocks: func(conn *mockldapconn.MockConn) { @@ -746,10 +748,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }, }}, - wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: { - UserSearchBase: testUserSearchBase, - GroupSearchBase: testGroupSearchBase, - }}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{}, }, { 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", @@ -759,8 +758,14 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { ldapConnectionValidTrueCondition(1234, "4242"), } })}, - inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{ + testName: {BindSecretResourceVersion: "4242", + LDAPConnectionProtocol: upstreamldap.TLS, + UserSearchBase: testUserSearchBase, + GroupSearchBase: testGroupSearchBase, + Generation: 1234, + }}, 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. }, @@ -777,6 +782,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}}, { 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", @@ -786,8 +792,14 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { ldapConnectionValidTrueCondition(1234, "4242"), } })}, - inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: { + BindSecretResourceVersion: "4242", + LDAPConnectionProtocol: upstreamldap.StartTLS, + UserSearchBase: testUserSearchBase, + GroupSearchBase: testGroupSearchBase, + Generation: 1234, + }}, 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. }, @@ -804,6 +816,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}}, { name: "when the LDAP server connection was validated for an older resource generation, then try to validate it again", @@ -813,8 +826,14 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { ldapConnectionValidTrueCondition(1233, "4242"), // older spec generation! } })}, - inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: { + BindSecretResourceVersion: "4242", + LDAPConnectionProtocol: upstreamldap.TLS, + Generation: 1233, + UserSearchBase: testUserSearchBase, + GroupSearchBase: testGroupSearchBase, + }}, setupMocks: func(conn *mockldapconn.MockConn) { // Should perform a test dial and bind. conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) @@ -833,6 +852,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}}, { name: "when the LDAP server connection validation previously failed for this resource generation, then try to validate it again", @@ -849,8 +869,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, } })}, - inputSecrets: []runtime.Object{validBindUserSecret("4242")}, - initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, setupMocks: func(conn *mockldapconn.MockConn) { // Should perform a test dial and bind. conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) @@ -869,7 +888,49 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - }}}, + Generation: 1234, + }}}, { + name: "when the validated settings cache is incomplete, then try to validate it again", + // this shouldn't happen, but if it does, just throw it out and try again. + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) { + upstream.Generation = 1234 + upstream.Status.Conditions = []v1alpha1.Condition{ + { + Type: "LDAPConnectionValid", + Status: "False", // failure! + LastTransitionTime: now, + Reason: "LDAPConnectionError", + Message: "some-error-message", + ObservedGeneration: 1234, // same (current) generation! + }, + } + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + 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) + conn.EXPECT().Close().Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstreamWithTLS}, + wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.LDAPIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: { + BindSecretResourceVersion: "4242", + LDAPConnectionProtocol: upstreamldap.TLS, + UserSearchBase: testUserSearchBase, + GroupSearchBase: testGroupSearchBase, + Generation: 1234, + }}, + }, { name: "when the LDAP server connection was already validated for this resource generation but the bind secret has changed, then try to validate it again", inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) { @@ -878,8 +939,14 @@ 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]upstreamwatchers.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, + UserSearchBase: testUserSearchBase, + GroupSearchBase: testGroupSearchBase, + Generation: 1234, + }}, // old version was validated setupMocks: func(conn *mockldapconn.MockConn) { // Should perform a test dial and bind. conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) @@ -898,6 +965,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, + Generation: 1234, }}}, } @@ -933,9 +1001,15 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { return conn, nil })} - validatedSecretVersionCache := upstreamwatchers.NewSecretVersionCache() + var validatedSecretVersionCache *upstreamwatchers.SecretVersionCache if tt.initialValidatedSettings != nil { - validatedSecretVersionCache.ValidatedSettingsByName = tt.initialValidatedSettings + validatedSecretVersionCache = &upstreamwatchers.SecretVersionCache{ + ValidatedSettingsByName: tt.initialValidatedSettings, + } + } else { + validatedSecretVersionCache = &upstreamwatchers.SecretVersionCache{ + ValidatedSettingsByName: map[string]upstreamwatchers.ValidatedSettings{}, + } } controller := newInternal( diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index 3801cb21..60d09eba 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go @@ -46,19 +46,40 @@ const ( // An in-memory cache with an entry for each ActiveDirectoryIdentityProvider, to keep track of which ResourceVersion // of the bind Secret, which TLS/StartTLS setting was used and which search base was found during the most recent successful validation. +type SecretVersionCacheI interface { + Get(upstreamName, resourceVersion string, generation int64) (ValidatedSettings, bool) + Set(upstreamName, resourceVersion string, generation int64, settings ValidatedSettings) +} + type SecretVersionCache struct { ValidatedSettingsByName map[string]ValidatedSettings } +func (s *SecretVersionCache) Get(upstreamName, resourceVersion string, generation int64) (ValidatedSettings, bool) { + validatedSettings := s.ValidatedSettingsByName[upstreamName] + if validatedSettings.BindSecretResourceVersion == resourceVersion && + validatedSettings.Generation == generation && validatedSettings.UserSearchBase != "" && + validatedSettings.GroupSearchBase != "" && validatedSettings.LDAPConnectionProtocol != "" { + return validatedSettings, true + } + return ValidatedSettings{}, false +} + +func (s *SecretVersionCache) Set(upstreamName, resourceVersion string, generation int64, settings ValidatedSettings) { + s.ValidatedSettingsByName[upstreamName] = settings +} + type ValidatedSettings struct { + Generation int64 BindSecretResourceVersion string LDAPConnectionProtocol upstreamldap.LDAPConnectionProtocol UserSearchBase string GroupSearchBase string } -func NewSecretVersionCache() *SecretVersionCache { - return &SecretVersionCache{ValidatedSettingsByName: map[string]ValidatedSettings{}} +func NewSecretVersionCache() SecretVersionCacheI { + cache := SecretVersionCache{ValidatedSettingsByName: map[string]ValidatedSettings{}} + return &cache } // read only interface for sharing between ldap and active directory. @@ -167,37 +188,6 @@ func TestConnection( } } -func HasPreviousSuccessfulTLSConnectionConditionForCurrentSpecGenerationAndSecretVersion(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 HasPreviousSuccessfulSearchBaseConditionForCurrentGeneration(secretVersionCache *SecretVersionCache, currentGeneration int64, upstreamStatusConditions []v1alpha1.Condition, upstreamName string, currentSecretVersion string, config *upstreamldap.ProviderConfig) bool { - for _, cond := range upstreamStatusConditions { - if cond.Type == TypeSearchBaseFound && 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. - validatedSettings := secretVersionCache.ValidatedSettingsByName[upstreamName] - // Reload the user search and group search base settings that were previously validated. - config.UserSearch.Base = validatedSettings.UserSearchBase - config.GroupSearch.Base = validatedSettings.GroupSearchBase - return true - } - } - return false -} - func validTLSCondition(message string) *v1alpha1.Condition { return &v1alpha1.Condition{ Type: typeTLSConfigurationValid, @@ -279,7 +269,7 @@ type GradatedCondition struct { isFatal bool } -func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, secretInformer corev1informers.SecretInformer, validatedSecretVersionsCache *SecretVersionCache, config *upstreamldap.ProviderConfig) GradatedConditions { +func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, secretInformer corev1informers.SecretInformer, validatedSecretVersionsCache SecretVersionCacheI, config *upstreamldap.ProviderConfig) GradatedConditions { conditions := GradatedConditions{} secretValidCondition, currentSecretVersion := ValidateSecret(secretInformer, upstream.Spec().BindSecretName(), upstream.Namespace(), config) conditions.Append(secretValidCondition, true) @@ -301,35 +291,44 @@ func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, s return conditions } -func validateAndSetLDAPServerConnectivityAndSearchBase(ctx context.Context, validatedSecretVersionsCache *SecretVersionCache, upstream UpstreamGenericLDAPIDP, config *upstreamldap.ProviderConfig, currentSecretVersion string) (*v1alpha1.Condition, *v1alpha1.Condition) { - var ldapConnectionValidCondition *v1alpha1.Condition - if !HasPreviousSuccessfulTLSConnectionConditionForCurrentSpecGenerationAndSecretVersion(validatedSecretVersionsCache, upstream.Generation(), upstream.Status().Conditions(), upstream.Name(), currentSecretVersion, config) { +func validateAndSetLDAPServerConnectivityAndSearchBase(ctx context.Context, validatedSecretVersionsCache SecretVersionCacheI, upstream UpstreamGenericLDAPIDP, config *upstreamldap.ProviderConfig, currentSecretVersion string) (*v1alpha1.Condition, *v1alpha1.Condition) { + // previouslyValidatedSecretVersion := validatedSecretVersionsCache.ValidatedSettingsByName[upstream.Name()].BindSecretResourceVersion + // doesn't have an existing entry for ValidatedSettingsByName with this secret version -> + // lets double check tls connection + // if we can connect, put it in the secret cache + // also we KNOW we need to recheck the search base stuff too... so they should all be one function? + // but if tls validation fails no need to also try to get search base stuff? + + validatedSettings, hasPreviousValidatedSettings := validatedSecretVersionsCache.Get(upstream.Name(), currentSecretVersion, upstream.Generation()) + var ldapConnectionValidCondition, searchBaseFoundCondition *v1alpha1.Condition + if !hasPreviousValidatedSettings { testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, probeLDAPTimeout) defer cancelFunc() ldapConnectionValidCondition = TestConnection(testConnectionTimeout, upstream.Spec().BindSecretName(), config, currentSecretVersion) - if ldapConnectionValidCondition.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, - } - } - } - var searchBaseFoundCondition *v1alpha1.Condition - if !HasPreviousSuccessfulSearchBaseConditionForCurrentGeneration(validatedSecretVersionsCache, upstream.Generation(), upstream.Status().Conditions(), upstream.Name(), currentSecretVersion, config) { searchBaseTimeout, cancelFunc := context.WithTimeout(ctx, probeLDAPTimeout) defer cancelFunc() - searchBaseFoundCondition = upstream.Spec().DetectAndSetSearchBase(searchBaseTimeout, config) - validatedSettings := validatedSecretVersionsCache.ValidatedSettingsByName[upstream.Name()] - validatedSettings.GroupSearchBase = config.GroupSearch.Base - validatedSettings.UserSearchBase = config.UserSearch.Base - validatedSecretVersionsCache.ValidatedSettingsByName[upstream.Name()] = validatedSettings + if ldapConnectionValidCondition.Status == v1alpha1.ConditionTrue { + // if it's nil, don't worry about the search base condition. But if it exists make sure the status is true. + if searchBaseFoundCondition == nil || (searchBaseFoundCondition.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. + validatedSettings.LDAPConnectionProtocol = config.ConnectionProtocol + validatedSettings.BindSecretResourceVersion = currentSecretVersion + validatedSettings.Generation = upstream.Generation() + validatedSettings.UserSearchBase = config.UserSearch.Base + validatedSettings.GroupSearchBase = config.GroupSearch.Base + validatedSecretVersionsCache.Set(upstream.Name(), currentSecretVersion, upstream.Generation(), validatedSettings) + } + } + } else { + config.ConnectionProtocol = validatedSettings.LDAPConnectionProtocol + config.UserSearch.Base = validatedSettings.UserSearchBase + config.GroupSearch.Base = validatedSettings.GroupSearchBase } return ldapConnectionValidCondition, searchBaseFoundCondition diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 504f4fef..b79d5255 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/oauth2" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" @@ -327,6 +328,188 @@ func TestSupervisorLogin(t *testing.T) { wantErrorDescription: "The resource owner or authorization server denied the request. Username/password not accepted by LDAP provider.", wantErrorType: "access_denied", }, + { + name: "ldap login still works after updating bind secret", + maybeSkip: func(t *testing.T) { + t.Helper() + if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) { + t.Skip("LDAP integration test requires connectivity to an LDAP server") + } + }, + createIDP: func(t *testing.T) { + t.Helper() + + secret := testlib.CreateTestSecret(t, env.SupervisorNamespace, "ldap-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamLDAP.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamLDAP.BindPassword, + }, + ) + secretName := secret.Name + ldapIDP := testlib.CreateTestLDAPIdentityProvider(t, idpv1alpha1.LDAPIdentityProviderSpec{ + Host: env.SupervisorUpstreamLDAP.Host, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.CABundle)), + }, + Bind: idpv1alpha1.LDAPIdentityProviderBind{ + SecretName: secretName, + }, + UserSearch: idpv1alpha1.LDAPIdentityProviderUserSearch{ + Base: env.SupervisorUpstreamLDAP.UserSearchBase, + Filter: "", + Attributes: idpv1alpha1.LDAPIdentityProviderUserSearchAttributes{ + Username: env.SupervisorUpstreamLDAP.TestUserMailAttributeName, + UID: env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeName, + }, + }, + GroupSearch: idpv1alpha1.LDAPIdentityProviderGroupSearch{ + Base: env.SupervisorUpstreamLDAP.GroupSearchBase, + Filter: "", + Attributes: idpv1alpha1.LDAPIdentityProviderGroupSearchAttributes{ + GroupName: "dn", + }, + }, + }, idpv1alpha1.LDAPPhaseReady) + + secret.Annotations = map[string]string{"pinniped.dev/test": "", "another-label": "another-key"} + // update that secret, which will cause the cache to recheck tls and search base values + client := testlib.NewKubernetesClientset(t) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + updatedSecret, err := client.CoreV1().Secrets(env.SupervisorNamespace).Update(ctx, secret, metav1.UpdateOptions{}) + require.NoError(t, err) + + 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, + updatedSecret.Name, updatedSecret.ResourceVersion, + ) + supervisorClient := testlib.NewSupervisorClientset(t) + testlib.RequireEventually(t, func(requireEventually *require.Assertions) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + ldapIDP, err = supervisorClient.IDPV1alpha1().LDAPIdentityProviders(env.SupervisorNamespace).Get(ctx, ldapIDP.Name, metav1.GetOptions{}) + requireEventually.NoError(err) + requireEventuallySuccessfulLDAPIdentityProviderConditions(t, requireEventually, ldapIDP, expectedMsg) + }, time.Minute, 500*time.Millisecond) + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login + env.SupervisorUpstreamLDAP.TestUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + // 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+ + "?base="+url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase)+ + "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), + ) + "$", + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$", + wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, + }, + { + name: "ldap login still works after deleting and recreating the bind secret", + maybeSkip: func(t *testing.T) { + t.Helper() + if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) { + t.Skip("LDAP integration test requires connectivity to an LDAP server") + } + }, + createIDP: func(t *testing.T) { + t.Helper() + + secret := testlib.CreateTestSecret(t, env.SupervisorNamespace, "ldap-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamLDAP.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamLDAP.BindPassword, + }, + ) + secretName := secret.Name + ldapIDP := testlib.CreateTestLDAPIdentityProvider(t, idpv1alpha1.LDAPIdentityProviderSpec{ + Host: env.SupervisorUpstreamLDAP.Host, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.CABundle)), + }, + Bind: idpv1alpha1.LDAPIdentityProviderBind{ + SecretName: secretName, + }, + UserSearch: idpv1alpha1.LDAPIdentityProviderUserSearch{ + Base: env.SupervisorUpstreamLDAP.UserSearchBase, + Filter: "", + Attributes: idpv1alpha1.LDAPIdentityProviderUserSearchAttributes{ + Username: env.SupervisorUpstreamLDAP.TestUserMailAttributeName, + UID: env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeName, + }, + }, + GroupSearch: idpv1alpha1.LDAPIdentityProviderGroupSearch{ + Base: env.SupervisorUpstreamLDAP.GroupSearchBase, + Filter: "", + Attributes: idpv1alpha1.LDAPIdentityProviderGroupSearchAttributes{ + GroupName: "dn", + }, + }, + }, idpv1alpha1.LDAPPhaseReady) + + // delete, then recreate that secret, which will cause the cache to recheck tls and search base values + client := testlib.NewKubernetesClientset(t) + deleteCtx, deleteCancel := context.WithTimeout(context.Background(), time.Minute) + defer deleteCancel() + err := client.CoreV1().Secrets(env.SupervisorNamespace).Delete(deleteCtx, secretName, metav1.DeleteOptions{}) + require.NoError(t, err) + + // create the secret again + recreateCtx, recreateCancel := context.WithTimeout(context.Background(), time.Minute) + defer recreateCancel() + recreatedSecret, err := client.CoreV1().Secrets(env.SupervisorNamespace).Create(recreateCtx, &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: env.SupervisorNamespace, + }, + Type: v1.SecretTypeBasicAuth, + StringData: map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamLDAP.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamLDAP.BindPassword, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + 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, + recreatedSecret.Name, recreatedSecret.ResourceVersion, + ) + supervisorClient := testlib.NewSupervisorClientset(t) + testlib.RequireEventually(t, func(requireEventually *require.Assertions) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + ldapIDP, err = supervisorClient.IDPV1alpha1().LDAPIdentityProviders(env.SupervisorNamespace).Get(ctx, ldapIDP.Name, metav1.GetOptions{}) + requireEventually.NoError(err) + requireEventuallySuccessfulLDAPIdentityProviderConditions(t, requireEventually, ldapIDP, expectedMsg) + }, time.Minute, 500*time.Millisecond) + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, // username to present to server during login + env.SupervisorUpstreamLDAP.TestUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + // 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+ + "?base="+url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase)+ + "&sub="+base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), + ) + "$", + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue) + "$", + wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs, + }, { name: "activedirectory with all default options", maybeSkip: func(t *testing.T) { @@ -448,6 +631,165 @@ func TestSupervisorLogin(t *testing.T) { wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserMailAttributeValue) + "$", wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserDirectGroupsDNs, }, + { + name: "active directory login still works after updating bind secret", + maybeSkip: func(t *testing.T) { + t.Helper() + if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) { + t.Skip("LDAP integration test requires connectivity to an LDAP server") + } + if env.SupervisorUpstreamActiveDirectory.Host == "" { + t.Skip("Active Directory hostname not specified") + } + }, + createIDP: func(t *testing.T) { + t.Helper() + + secret := testlib.CreateTestSecret(t, env.SupervisorNamespace, "ad-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamActiveDirectory.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamActiveDirectory.BindPassword, + }, + ) + secretName := secret.Name + adIDP := testlib.CreateTestActiveDirectoryIdentityProvider(t, idpv1alpha1.ActiveDirectoryIdentityProviderSpec{ + Host: env.SupervisorUpstreamActiveDirectory.Host, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)), + }, + Bind: idpv1alpha1.ActiveDirectoryIdentityProviderBind{ + SecretName: secretName, + }, + }, idpv1alpha1.ActiveDirectoryPhaseReady) + + secret.Annotations = map[string]string{"pinniped.dev/test": "", "another-label": "another-key"} + // update that secret, which will cause the cache to recheck tls and search base values + client := testlib.NewKubernetesClientset(t) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + updatedSecret, err := client.CoreV1().Secrets(env.SupervisorNamespace).Update(ctx, secret, metav1.UpdateOptions{}) + require.NoError(t, err) + + expectedMsg := fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + env.SupervisorUpstreamActiveDirectory.Host, env.SupervisorUpstreamActiveDirectory.BindUsername, + updatedSecret.Name, updatedSecret.ResourceVersion, + ) + supervisorClient := testlib.NewSupervisorClientset(t) + testlib.RequireEventually(t, func(requireEventually *require.Assertions) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + adIDP, err = supervisorClient.IDPV1alpha1().ActiveDirectoryIdentityProviders(env.SupervisorNamespace).Get(ctx, adIDP.Name, metav1.GetOptions{}) + requireEventually.NoError(err) + requireEventuallySuccessfulActiveDirectoryIdentityProviderConditions(t, requireEventually, adIDP, expectedMsg) + }, time.Minute, 500*time.Millisecond) + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue, // username to present to server during login + env.SupervisorUpstreamActiveDirectory.TestUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + // 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.SupervisorUpstreamActiveDirectory.Host+ + "?base="+url.QueryEscape(env.SupervisorUpstreamActiveDirectory.DefaultNamingContextSearchBase)+ + "&sub="+env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue, + ) + "$", + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$", + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, + }, + { + name: "active directory login still works after deleting and recreating bind secret", + maybeSkip: func(t *testing.T) { + t.Helper() + if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) { + t.Skip("LDAP integration test requires connectivity to an LDAP server") + } + if env.SupervisorUpstreamActiveDirectory.Host == "" { + t.Skip("Active Directory hostname not specified") + } + }, + createIDP: func(t *testing.T) { + t.Helper() + + secret := testlib.CreateTestSecret(t, env.SupervisorNamespace, "ad-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamActiveDirectory.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamActiveDirectory.BindPassword, + }, + ) + secretName := secret.Name + adIDP := testlib.CreateTestActiveDirectoryIdentityProvider(t, idpv1alpha1.ActiveDirectoryIdentityProviderSpec{ + Host: env.SupervisorUpstreamActiveDirectory.Host, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)), + }, + Bind: idpv1alpha1.ActiveDirectoryIdentityProviderBind{ + SecretName: secretName, + }, + }, idpv1alpha1.ActiveDirectoryPhaseReady) + + // delete the secret + client := testlib.NewKubernetesClientset(t) + deleteCtx, deleteCancel := context.WithTimeout(context.Background(), time.Minute) + defer deleteCancel() + err := client.CoreV1().Secrets(env.SupervisorNamespace).Delete(deleteCtx, secretName, metav1.DeleteOptions{}) + require.NoError(t, err) + + // create the secret again + recreateCtx, recreateCancel := context.WithTimeout(context.Background(), time.Minute) + defer recreateCancel() + recreatedSecret, err := client.CoreV1().Secrets(env.SupervisorNamespace).Create(recreateCtx, &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: env.SupervisorNamespace, + }, + Type: v1.SecretTypeBasicAuth, + StringData: map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamActiveDirectory.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamActiveDirectory.BindPassword, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + + expectedMsg := fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + env.SupervisorUpstreamActiveDirectory.Host, env.SupervisorUpstreamActiveDirectory.BindUsername, + recreatedSecret.Name, recreatedSecret.ResourceVersion, + ) + supervisorClient := testlib.NewSupervisorClientset(t) + testlib.RequireEventually(t, func(requireEventually *require.Assertions) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + adIDP, err = supervisorClient.IDPV1alpha1().ActiveDirectoryIdentityProviders(env.SupervisorNamespace).Get(ctx, adIDP.Name, metav1.GetOptions{}) + requireEventually.NoError(err) + requireEventuallySuccessfulActiveDirectoryIdentityProviderConditions(t, requireEventually, adIDP, expectedMsg) + }, time.Minute, 500*time.Millisecond) + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorizationUsingCLIPasswordFlow(t, + downstreamAuthorizeURL, + env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue, // username to present to server during login + env.SupervisorUpstreamActiveDirectory.TestUserPassword, // password to present to server during login + httpClient, + false, + ) + }, + // 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.SupervisorUpstreamActiveDirectory.Host+ + "?base="+url.QueryEscape(env.SupervisorUpstreamActiveDirectory.DefaultNamingContextSearchBase)+ + "&sub="+env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue, + ) + "$", + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue) + "$", + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, + }, { name: "logging in to activedirectory with a deactivated user fails", maybeSkip: func(t *testing.T) { @@ -570,6 +912,66 @@ func requireSuccessfulActiveDirectoryIdentityProviderConditions(t *testing.T, ad }, conditionsSummary) } +func requireEventuallySuccessfulLDAPIdentityProviderConditions(t *testing.T, requireEventually *require.Assertions, ldapIDP *idpv1alpha1.LDAPIdentityProvider, expectedLDAPConnectionValidMessage string) { + t.Helper() + requireEventually.Len(ldapIDP.Status.Conditions, 3) + + conditionsSummary := [][]string{} + for _, condition := range ldapIDP.Status.Conditions { + conditionsSummary = append(conditionsSummary, []string{condition.Type, string(condition.Status), condition.Reason}) + t.Logf("Saw ActiveDirectoryIdentityProvider Status.Condition Type=%s Status=%s Reason=%s Message=%s", + condition.Type, string(condition.Status), condition.Reason, condition.Message) + switch condition.Type { + case "BindSecretValid": + requireEventually.Equal("loaded bind secret", condition.Message) + case "TLSConfigurationValid": + requireEventually.Equal("loaded TLS configuration", condition.Message) + case "LDAPConnectionValid": + requireEventually.Equal(expectedLDAPConnectionValidMessage, condition.Message) + } + } + + requireEventually.ElementsMatch([][]string{ + {"BindSecretValid", "True", "Success"}, + {"TLSConfigurationValid", "True", "Success"}, + {"LDAPConnectionValid", "True", "Success"}, + }, conditionsSummary) +} + +func requireEventuallySuccessfulActiveDirectoryIdentityProviderConditions(t *testing.T, requireEventually *require.Assertions, adIDP *idpv1alpha1.ActiveDirectoryIdentityProvider, expectedActiveDirectoryConnectionValidMessage string) { + t.Helper() + requireEventually.Len(adIDP.Status.Conditions, 4) + + conditionsSummary := [][]string{} + for _, condition := range adIDP.Status.Conditions { + conditionsSummary = append(conditionsSummary, []string{condition.Type, string(condition.Status), condition.Reason}) + t.Logf("Saw ActiveDirectoryIdentityProvider Status.Condition Type=%s Status=%s Reason=%s Message=%s", + condition.Type, string(condition.Status), condition.Reason, condition.Message) + switch condition.Type { + case "BindSecretValid": + requireEventually.Equal("loaded bind secret", condition.Message) + case "TLSConfigurationValid": + requireEventually.Equal("loaded TLS configuration", condition.Message) + case "LDAPConnectionValid": + requireEventually.Equal(expectedActiveDirectoryConnectionValidMessage, condition.Message) + } + } + + expectedUserSearchReason := "" + if adIDP.Spec.UserSearch.Base == "" || adIDP.Spec.GroupSearch.Base == "" { + expectedUserSearchReason = "Success" + } else { + expectedUserSearchReason = "UsingConfigurationFromSpec" + } + + requireEventually.ElementsMatch([][]string{ + {"BindSecretValid", "True", "Success"}, + {"TLSConfigurationValid", "True", "Success"}, + {"LDAPConnectionValid", "True", "Success"}, + {"SearchBaseFound", "True", expectedUserSearchReason}, + }, conditionsSummary) +} + func testSupervisorLogin( t *testing.T, createIDP func(t *testing.T), diff --git a/test/testlib/client.go b/test/testlib/client.go index 9345ffa3..d75f37d9 100644 --- a/test/testlib/client.go +++ b/test/testlib/client.go @@ -464,7 +464,7 @@ func CreateTestActiveDirectoryIdentityProvider(t *testing.T, spec idpv1alpha1.Ac }) t.Logf("created test ActiveDirectoryIdentityProvider %s", created.Name) - // Wait for the LDAPIdentityProvider to enter the expected phase (or time out). + // Wait for the ActiveDirectoryIdentityProvider to enter the expected phase (or time out). var result *idpv1alpha1.ActiveDirectoryIdentityProvider RequireEventuallyf(t, func(requireEventually *require.Assertions) {