diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index bdc9ec42..65328e54 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -392,6 +392,7 @@ export PINNIPED_TEST_AD_USER_PASSWORD="$(gcloud secrets versions access latest - export PINNIPED_TEST_AD_LDAPS_CA_BUNDLE="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-ca-data' -)" export PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_DN="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-expected-direct-groups-dn' -)" export PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_CN="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-expected-direct-groups-cn' -)" +export PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_SAMACCOUNTNAME="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-expected-direct-and-nested-groups-samaccountnames' -)" fi read -r -d '' PINNIPED_TEST_CLUSTER_CAPABILITY_YAML << PINNIPED_TEST_CLUSTER_CAPABILITY_YAML_EOF || true diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 46280b21..c6dcd0e7 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -43,7 +43,7 @@ const ( // - is a group. // - has a member that matches the DN of the user we successfully logged in as. // - perform nested group search by default. - defaultActiveDirectoryGroupSearchFilter = "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={})" + defaultActiveDirectoryGroupSearchFilter = "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))" ) type activeDirectoryUpstreamGenericLDAPImpl struct { @@ -142,14 +142,23 @@ func (u *activeDirectoryUpstreamGenericLDAPUserSearch) Base() string { } func (u *activeDirectoryUpstreamGenericLDAPUserSearch) Filter() string { + if len(u.userSearch.Filter) == 0 { + return defaultActiveDirectoryUserSearchFilter + } return u.userSearch.Filter } func (u *activeDirectoryUpstreamGenericLDAPUserSearch) UsernameAttribute() string { + if len(u.userSearch.Attributes.Username) == 0 { + return defaultActiveDirectoryUsernameAttributeName + } return u.userSearch.Attributes.Username } func (u *activeDirectoryUpstreamGenericLDAPUserSearch) UIDAttribute() string { + if len(u.userSearch.Attributes.UID) == 0 { + return defaultActiveDirectoryUIDAttributeName + } return u.userSearch.Attributes.UID } @@ -162,10 +171,16 @@ func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) Base() string { } func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) Filter() string { + if len(g.groupSearch.Filter) == 0 { + return defaultActiveDirectoryGroupSearchFilter + } return g.groupSearch.Filter } func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) GroupNameAttribute() string { + if len(g.groupSearch.Attributes.GroupName) == 0 { + return defaultActiveDirectoryGroupNameAttributeName + } return g.groupSearch.Attributes.GroupName } @@ -275,48 +290,26 @@ func (c *activeDirectoryWatcherController) Sync(ctx controllerlib.Context) error func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider) (p provider.UpstreamLDAPIdentityProviderI, requeue bool) { spec := upstream.Spec - usernameAttribute := spec.UserSearch.Attributes.Username - if len(usernameAttribute) == 0 { - usernameAttribute = defaultActiveDirectoryUsernameAttributeName - } - uidAttribute := spec.UserSearch.Attributes.UID - if len(uidAttribute) == 0 { - uidAttribute = defaultActiveDirectoryUIDAttributeName - } - - groupNameAttribute := spec.GroupSearch.Attributes.GroupName - if len(groupNameAttribute) == 0 { - groupNameAttribute = defaultActiveDirectoryGroupNameAttributeName - } - - userSearchFilter := spec.UserSearch.Filter - if len(userSearchFilter) == 0 { - userSearchFilter = defaultActiveDirectoryUserSearchFilter - } - - groupSearchFilter := spec.GroupSearch.Filter - if len(groupSearchFilter) == 0 { - groupSearchFilter = defaultActiveDirectoryGroupSearchFilter - } + adUpstreamImpl := activeDirectoryUpstreamGenericLDAPImpl{*upstream} config := &upstreamldap.ProviderConfig{ Name: upstream.Name, Host: spec.Host, UserSearch: upstreamldap.UserSearchConfig{ Base: spec.UserSearch.Base, - Filter: userSearchFilter, - UsernameAttribute: usernameAttribute, - UIDAttribute: uidAttribute, + Filter: adUpstreamImpl.Spec().UserSearch().Filter(), + UsernameAttribute: adUpstreamImpl.Spec().UserSearch().UsernameAttribute(), + UIDAttribute: adUpstreamImpl.Spec().UserSearch().UIDAttribute(), }, GroupSearch: upstreamldap.GroupSearchConfig{ Base: spec.GroupSearch.Base, - Filter: groupSearchFilter, - GroupNameAttribute: groupNameAttribute, + Filter: adUpstreamImpl.Spec().GroupSearch().Filter(), + GroupNameAttribute: adUpstreamImpl.Spec().GroupSearch().GroupNameAttribute(), }, Dialer: c.ldapDialer, } - conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &activeDirectoryUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.validatedSecretVersionsCache, config) + conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &adUpstreamImpl, c.secretInformer, c.validatedSecretVersionsCache, config) c.updateStatus(ctx, upstream, conditions.Conditions()) 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 d05de468..6d11d0cb 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -1183,7 +1183,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { }, GroupSearch: upstreamldap.GroupSearchConfig{ Base: testGroupSearchBase, - Filter: "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={})", + Filter: "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))", GroupNameAttribute: "sAMAccountName", }, }, diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 6a556a29..2da945b2 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -281,7 +281,7 @@ func TestSupervisorLogin(t *testing.T) { ), // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUserSAMAccountNameValue), - wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserDirectGroupsDNs, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountNames, }, } for _, test := range tests { diff --git a/test/testlib/env.go b/test/testlib/env.go index 04c770e0..ee83e1c7 100644 --- a/test/testlib/env.go +++ b/test/testlib/env.go @@ -82,23 +82,24 @@ 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"` - UserSearchBase string `json:"userSearchBase"` - GroupSearchBase string `json:"groupSearchBase"` - TestUserDN string `json:"testUserDN"` - TestUserCN string `json:"testUserCN"` - TestUserPassword string `json:"testUserPassword"` - TestUserMailAttributeName string `json:"testUserMailAttributeName"` - TestUserMailAttributeValue string `json:"testUserMailAttributeValue"` - TestUserUniqueIDAttributeName string `json:"testUserUniqueIDAttributeName"` - TestUserUniqueIDAttributeValue string `json:"testUserUniqueIDAttributeValue"` - TestUserDirectGroupsCNs []string `json:"testUserDirectGroupsCNs"` - TestUserDirectGroupsDNs []string `json:"testUserDirectGroupsDNs"` //nolint:golint // this is "distinguished names", not "DNS" - TestUserSAMAccountNameValue string `json:"testUserSAMAccountNameValue"` + Host string `json:"host"` + StartTLSOnlyHost string `json:"startTLSOnlyHost"` + CABundle string `json:"caBundle"` + BindUsername string `json:"bindUsername"` + BindPassword string `json:"bindPassword"` + UserSearchBase string `json:"userSearchBase"` + GroupSearchBase string `json:"groupSearchBase"` + TestUserDN string `json:"testUserDN"` + TestUserCN string `json:"testUserCN"` + TestUserPassword string `json:"testUserPassword"` + TestUserMailAttributeName string `json:"testUserMailAttributeName"` + TestUserMailAttributeValue string `json:"testUserMailAttributeValue"` + TestUserUniqueIDAttributeName string `json:"testUserUniqueIDAttributeName"` + TestUserUniqueIDAttributeValue string `json:"testUserUniqueIDAttributeValue"` + TestUserDirectGroupsCNs []string `json:"testUserDirectGroupsCNs"` + TestUserDirectGroupsDNs []string `json:"testUserDirectGroupsDNs"` //nolint:golint // this is "distinguished names", not "DNS" + TestUserSAMAccountNameValue string `json:"testUserSAMAccountNameValue"` + TestUserIndirectGroupsSAMAccountNames []string `json:"TestUserIndirectGroupsSAMAccountNames"` } // ProxyEnv returns a set of environment variable strings (e.g., to combine with os.Environ()) which set up the configured test HTTP proxy. @@ -270,22 +271,24 @@ func loadEnvVars(t *testing.T, result *TestEnv) { } result.SupervisorUpstreamActiveDirectory = TestLDAPUpstream{ - Host: wantEnv("PINNIPED_TEST_AD_HOST", ""), - CABundle: base64Decoded(t, os.Getenv("PINNIPED_TEST_AD_LDAPS_CA_BUNDLE")), - BindUsername: wantEnv("PINNIPED_TEST_AD_BIND_ACCOUNT_USERNAME", ""), - BindPassword: wantEnv("PINNIPED_TEST_AD_BIND_ACCOUNT_PASSWORD", ""), - TestUserPassword: wantEnv("PINNIPED_TEST_AD_USER_PASSWORD", ""), - TestUserUniqueIDAttributeName: wantEnv("PINNIPED_TEST_AD_USER_UNIQUE_ID_ATTRIBUTE_NAME", ""), - TestUserUniqueIDAttributeValue: wantEnv("PINNIPED_TEST_AD_USER_UNIQUE_ID_ATTRIBUTE_VALUE", ""), - TestUserSAMAccountNameValue: wantEnv("PINNIPED_TEST_AD_USERNAME_ATTRIBUTE_VALUE", ""), - TestUserDirectGroupsDNs: filterEmpty(strings.Split(wantEnv("PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_DN", ""), ";")), - TestUserDirectGroupsCNs: filterEmpty(strings.Split(wantEnv("PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_CN", ""), ";")), + Host: wantEnv("PINNIPED_TEST_AD_HOST", ""), + CABundle: base64Decoded(t, os.Getenv("PINNIPED_TEST_AD_LDAPS_CA_BUNDLE")), + BindUsername: wantEnv("PINNIPED_TEST_AD_BIND_ACCOUNT_USERNAME", ""), + BindPassword: wantEnv("PINNIPED_TEST_AD_BIND_ACCOUNT_PASSWORD", ""), + TestUserPassword: wantEnv("PINNIPED_TEST_AD_USER_PASSWORD", ""), + TestUserUniqueIDAttributeName: wantEnv("PINNIPED_TEST_AD_USER_UNIQUE_ID_ATTRIBUTE_NAME", ""), + TestUserUniqueIDAttributeValue: wantEnv("PINNIPED_TEST_AD_USER_UNIQUE_ID_ATTRIBUTE_VALUE", ""), + TestUserSAMAccountNameValue: wantEnv("PINNIPED_TEST_AD_USERNAME_ATTRIBUTE_VALUE", ""), + TestUserDirectGroupsDNs: filterEmpty(strings.Split(wantEnv("PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_DN", ""), ";")), + TestUserDirectGroupsCNs: filterEmpty(strings.Split(wantEnv("PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_CN", ""), ";")), + TestUserIndirectGroupsSAMAccountNames: filterEmpty(strings.Split(wantEnv("PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_SAMACCOUNTNAME", ""), ";")), } sort.Strings(result.SupervisorUpstreamLDAP.TestUserDirectGroupsCNs) sort.Strings(result.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs) sort.Strings(result.SupervisorUpstreamActiveDirectory.TestUserDirectGroupsCNs) sort.Strings(result.SupervisorUpstreamActiveDirectory.TestUserDirectGroupsDNs) + sort.Strings(result.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountNames) } func (e *TestEnv) HasCapability(cap Capability) bool {