From 26c47d564feecdf3e0c463b3be0b0aac45b1bed5 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Tue, 17 Aug 2021 16:53:26 -0700 Subject: [PATCH] Make new combined sAMAccountName@domain attribute the group name Also change default username attribute to userPrincipalName --- .../active_directory_upstream_watcher.go | 27 +++- .../active_directory_upstream_watcher_test.go | 85 ++++++++++-- internal/upstreamldap/upstreamldap.go | 49 ++++++- internal/upstreamldap/upstreamldap_test.go | 127 +++++++++++++++++- test/integration/supervisor_login_test.go | 2 +- test/testlib/env.go | 80 +++++------ 6 files changed, 307 insertions(+), 63 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 677ea2b7..b83b8910 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -29,16 +29,23 @@ const ( activeDirectoryControllerName = "active-directory-upstream-observer" // Default values for active directory config. - defaultActiveDirectoryUsernameAttributeName = "sAMAccountName" - defaultActiveDirectoryUIDAttributeName = "objectGUID" - defaultActiveDirectoryGroupNameAttributeName = "sAMAccountName" + defaultActiveDirectoryUsernameAttributeName = "userPrincipalName" + defaultActiveDirectoryUIDAttributeName = "objectGUID" + + // This is not a real attribute in active directory. + // It represents a combined attribute name, sAMAccountName + "@" + domain. + // For example if your group sAMAccountName is "mammals" and your domain is + // "activedirectory.example.com", it would be mammals@activedirectory.example.com. + // This is because sAMAccountName is only unique within a domain, not a forest. + defaultActiveDirectoryGroupNameAttributeName = "sAMAccountName" + defaultActiveDirectoryGroupNameOverrideAttributeName = "pinniped:sAMAccountName@domain" // - is a person. // - is not a computer. // - is not shown in advanced view only (which would likely mean its a system created service account with advanced permissions). - // - either the sAMAccountName or the mail attribute matches the input username. + // - either the sAMAccountName, the userPrincipalName or the mail attribute matches the input username. // - the sAMAccountType is for a normal user account. - defaultActiveDirectoryUserSearchFilter = "(&(objectClass=person)(!(objectClass=computer))(!(showInAdvancedViewOnly=TRUE))(|(sAMAccountName={})(mail={}))(sAMAccountType=805306368))" + defaultActiveDirectoryUserSearchFilter = "(&(objectClass=person)(!(objectClass=computer))(!(showInAdvancedViewOnly=TRUE))(|(sAMAccountName={})(mail={})(userPrincipalName={}))(sAMAccountType=805306368))" // - is a group. // - has a member that matches the DN of the user we successfully logged in as. @@ -181,6 +188,10 @@ func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) GroupNameAttribute() str if len(g.groupSearch.Attributes.GroupName) == 0 { return defaultActiveDirectoryGroupNameAttributeName } + // you explicitly told us to use the override value + if g.groupSearch.Attributes.GroupName == defaultActiveDirectoryGroupNameOverrideAttributeName { + return defaultActiveDirectoryGroupNameAttributeName + } return g.groupSearch.Attributes.GroupName } @@ -307,7 +318,11 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, GroupNameAttribute: adUpstreamImpl.Spec().GroupSearch().GroupNameAttribute(), }, Dialer: c.ldapDialer, - UIDAttributeParsingOverrides: []upstreamldap.AttributeParsingOverride{{AttributeName: "objectGUID", OverrideFunc: upstreamldap.MicrosoftUUIDFromBinary}}, + UIDAttributeParsingOverrides: []upstreamldap.AttributeParsingOverride{{AttributeName: "objectGUID", OverrideFunc: upstreamldap.MicrosoftUUIDFromBinary("objectGUID")}}, + } + + if spec.GroupSearch.Attributes.GroupName == defaultActiveDirectoryGroupNameOverrideAttributeName || spec.GroupSearch.Attributes.GroupName == "" { + config.GroupAttributeParsingOverrides = []upstreamldap.AttributeParsingOverride{{AttributeName: defaultActiveDirectoryGroupNameAttributeName, OverrideFunc: upstreamldap.GroupSAMAccountNameWithDomainSuffix}} } conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &adUpstreamImpl, c.secretInformer, c.validatedSecretVersionsCache, config) 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 d3657099..289e0d2f 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -1181,8 +1181,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { BindPassword: testBindPassword, UserSearch: upstreamldap.UserSearchConfig{ Base: testUserSearchBase, - Filter: "(&(objectClass=person)(!(objectClass=computer))(!(showInAdvancedViewOnly=TRUE))(|(sAMAccountName={})(mail={}))(sAMAccountType=805306368))", - UsernameAttribute: "sAMAccountName", + Filter: "(&(objectClass=person)(!(objectClass=computer))(!(showInAdvancedViewOnly=TRUE))(|(sAMAccountName={})(mail={})(userPrincipalName={}))(sAMAccountType=805306368))", + UsernameAttribute: "userPrincipalName", UIDAttribute: "objectGUID", }, GroupSearch: upstreamldap.GroupSearchConfig{ @@ -1190,7 +1190,59 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))", GroupNameAttribute: "sAMAccountName", }, - UIDAttributeParsingOverrides: []upstreamldap.AttributeParsingOverride{{AttributeName: "objectGUID", OverrideFunc: upstreamldap.MicrosoftUUIDFromBinary}}, + UIDAttributeParsingOverrides: []upstreamldap.AttributeParsingOverride{{AttributeName: "objectGUID", OverrideFunc: upstreamldap.MicrosoftUUIDFromBinary}}, + GroupAttributeParsingOverrides: []upstreamldap.AttributeParsingOverride{{AttributeName: "sAMAccountName", OverrideFunc: upstreamldap.GroupSAMAccountNameWithDomainSuffix}}, + }, + }, + wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{ + Phase: "Ready", + Conditions: allConditionsTrue(1234, "4242"), + }, + }}, + wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: { + BindSecretResourceVersion: "4242", + LDAPConnectionProtocol: upstreamldap.TLS, + UserSearchBase: testUserSearchBase, + GroupSearchBase: testGroupSearchBase, + }}, + }, + { + name: "when the input activedirectoryidentityprovider group search attributes is the special cased pinniped:sAMAccountName@domain value", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.ActiveDirectoryIdentityProvider) { + upstream.Spec.UserSearch.Attributes = v1alpha1.ActiveDirectoryIdentityProviderUserSearchAttributes{} + upstream.Spec.UserSearch.Filter = "" + upstream.Spec.GroupSearch.Filter = "" + upstream.Spec.GroupSearch.Attributes = v1alpha1.ActiveDirectoryIdentityProviderGroupSearchAttributes{GroupName: "pinniped:sAMAccountName@domain"} + })}, + 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) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{ + { + Name: testName, + Host: testHost, + ConnectionProtocol: upstreamldap.TLS, + CABundle: testCABundle, + BindUsername: testBindUsername, + BindPassword: testBindPassword, + UserSearch: upstreamldap.UserSearchConfig{ + Base: testUserSearchBase, + Filter: "(&(objectClass=person)(!(objectClass=computer))(!(showInAdvancedViewOnly=TRUE))(|(sAMAccountName={})(mail={})(userPrincipalName={}))(sAMAccountType=805306368))", + UsernameAttribute: "userPrincipalName", + UIDAttribute: "objectGUID", + }, + GroupSearch: upstreamldap.GroupSearchConfig{ + Base: testGroupSearchBase, + Filter: "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))", + GroupNameAttribute: "sAMAccountName", + }, + UIDAttributeParsingOverrides: []upstreamldap.AttributeParsingOverride{{AttributeName: "objectGUID", OverrideFunc: upstreamldap.MicrosoftUUIDFromBinary}}, + GroupAttributeParsingOverrides: []upstreamldap.AttributeParsingOverride{{AttributeName: "sAMAccountName", OverrideFunc: upstreamldap.GroupSAMAccountNameWithDomainSuffix}}, }, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1232,7 +1284,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { UserSearch: upstreamldap.UserSearchConfig{ Base: exampleDefaultNamingContext, Filter: testUserSearchFilter, - UsernameAttribute: "sAMAccountName", + UsernameAttribute: "userPrincipalName", UIDAttribute: "objectGUID", }, GroupSearch: upstreamldap.GroupSearchConfig{ @@ -1281,7 +1333,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { UserSearch: upstreamldap.UserSearchConfig{ Base: exampleDefaultNamingContext, Filter: testUserSearchFilter, - UsernameAttribute: "sAMAccountName", + UsernameAttribute: "userPrincipalName", UIDAttribute: "objectGUID", }, GroupSearch: upstreamldap.GroupSearchConfig{ @@ -1330,7 +1382,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { UserSearch: upstreamldap.UserSearchConfig{ Base: testUserSearchBase, Filter: testUserSearchFilter, - UsernameAttribute: "sAMAccountName", + UsernameAttribute: "userPrincipalName", UIDAttribute: "objectGUID", }, GroupSearch: upstreamldap.GroupSearchConfig{ @@ -1582,10 +1634,23 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { copyOfExpectedValueForResultingCache.UIDAttributeParsingOverrides = []upstreamldap.AttributeParsingOverride{} actualConfig.UIDAttributeParsingOverrides = []upstreamldap.AttributeParsingOverride{} - require.Len(t, actualUIDAttributeParsingOverrides, 1) - require.Len(t, expectedUIDAttributeParsingOverrides, 1) - require.Equal(t, expectedUIDAttributeParsingOverrides[0].AttributeName, actualUIDAttributeParsingOverrides[0].AttributeName) - require.Equal(t, reflect.ValueOf(expectedUIDAttributeParsingOverrides[0].OverrideFunc).Pointer(), reflect.ValueOf(actualUIDAttributeParsingOverrides[0].OverrideFunc).Pointer()) + require.Equal(t, len(expectedUIDAttributeParsingOverrides), len(actualUIDAttributeParsingOverrides)) + for i := range expectedUIDAttributeParsingOverrides { + require.Equal(t, expectedUIDAttributeParsingOverrides[i].AttributeName, actualUIDAttributeParsingOverrides[i].AttributeName) + require.Equal(t, reflect.ValueOf(expectedUIDAttributeParsingOverrides[i].OverrideFunc).Pointer(), reflect.ValueOf(actualUIDAttributeParsingOverrides[i].OverrideFunc).Pointer()) + } + + // function equality is awkward. Do the check for equality separately from the rest of the config. + expectedGroupAttributeParsingOverrides := copyOfExpectedValueForResultingCache.GroupAttributeParsingOverrides + actualGroupAttributeParsingOverrides := actualConfig.GroupAttributeParsingOverrides + copyOfExpectedValueForResultingCache.GroupAttributeParsingOverrides = []upstreamldap.AttributeParsingOverride{} + actualConfig.GroupAttributeParsingOverrides = []upstreamldap.AttributeParsingOverride{} + + require.Equal(t, len(expectedGroupAttributeParsingOverrides), len(actualGroupAttributeParsingOverrides)) + for i := range expectedGroupAttributeParsingOverrides { + require.Equal(t, expectedGroupAttributeParsingOverrides[i].AttributeName, actualGroupAttributeParsingOverrides[i].AttributeName) + require.Equal(t, reflect.ValueOf(expectedGroupAttributeParsingOverrides[i].OverrideFunc).Pointer(), reflect.ValueOf(actualGroupAttributeParsingOverrides[i].OverrideFunc).Pointer()) + } require.Equal(t, copyOfExpectedValueForResultingCache, actualConfig) } diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 89bd3338..b6e9571a 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -13,6 +13,7 @@ import ( "fmt" "net" "net/url" + "regexp" "sort" "strings" "time" @@ -106,14 +107,18 @@ type ProviderConfig struct { // Dialer exists to enable testing. When nil, will use a default appropriate for production use. Dialer LDAPDialer - // UIDAttributeParsingOverrides are mappings between an attribute name and a way to parse it when + // UIDAttributeParsingOverrides are mappings between an attribute name and a way to parse it as a UID when // it comes out of LDAP. UIDAttributeParsingOverrides []AttributeParsingOverride + + // GroupNameMappingOverrides are the mappings between an attribute name and a way to parse it as a group + // name when it comes out of LDAP. + GroupAttributeParsingOverrides []AttributeParsingOverride } type AttributeParsingOverride struct { AttributeName string - OverrideFunc func([]byte) (string, error) + OverrideFunc func(entry *ldap.Entry) (string, error) } // UserSearchConfig contains information about how to search for users in the upstream LDAP IDP. @@ -389,6 +394,15 @@ func (p *Provider) searchGroupsForUserDN(conn Conn, userDN string) ([]string, er if err != nil { return nil, fmt.Errorf(`error searching for group memberships for user with DN %q: %w`, userDN, err) } + for _, override := range p.c.GroupAttributeParsingOverrides { + if groupAttributeName == override.AttributeName { + overrideGroupName, err := override.OverrideFunc(groupEntry) + if err != nil { + return nil, fmt.Errorf("error finding groups: %w", err) + } + mappedGroupName = overrideGroupName + } + } groups = append(groups, mappedGroupName) } @@ -623,7 +637,7 @@ func (p *Provider) getSearchResultAttributeRawValueEncoded(attributeName string, for _, override := range p.c.UIDAttributeParsingOverrides { if attributeName == override.AttributeName { - return override.OverrideFunc(attributeValue) + return override.OverrideFunc(entry) } } @@ -671,7 +685,15 @@ func (p *Provider) traceSearchBaseDiscoveryFailure(t *trace.Trace, err error) { trace.Field{Key: "reason", Value: err.Error()}) } -func MicrosoftUUIDFromBinary(binaryUUID []byte) (string, error) { +func MicrosoftUUIDFromBinary(attributeName string) func(entry *ldap.Entry) (string, error) { + // validation has already been done so we can just get the attribute... + return func(entry *ldap.Entry) (string, error) { + binaryUUID := entry.GetRawAttributeValue(attributeName) + return microsoftUUIDFromBinary(binaryUUID) + } +} + +func microsoftUUIDFromBinary(binaryUUID []byte) (string, error) { uuidVal, err := uuid.FromBytes(binaryUUID) // start out with the RFC4122 version if err != nil { return "", err @@ -683,3 +705,22 @@ func MicrosoftUUIDFromBinary(binaryUUID []byte) (string, error) { uuidVal[6], uuidVal[7] = uuidVal[7], uuidVal[6] return uuidVal.String(), nil } + +func GroupSAMAccountNameWithDomainSuffix(entry *ldap.Entry) (string, error) { + sAMAccountNameAttribute := "sAMAccountName" + sAMAccountName := entry.GetAttributeValue(sAMAccountNameAttribute) + distinguishedName := entry.DN + domain, err := getDomainFromDistinguishedName(distinguishedName) + if err != nil { + return "", err + } + return sAMAccountName + "@" + domain, nil +} + +func getDomainFromDistinguishedName(distinguishedName string) (string, error) { + domainComponents := regexp.MustCompile(",DC=|,dc=").Split(distinguishedName, -1) + if len(domainComponents) == 1 { + return "", fmt.Errorf("did not find domain components in group dn: %s", distinguishedName) + } + return strings.Join(domainComponents[1:], "."), nil +} diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index 12e7f4eb..641bee40 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -513,7 +513,7 @@ func TestEndUserAuthentication(t *testing.T) { providerConfig: providerConfig(func(p *ProviderConfig) { p.UIDAttributeParsingOverrides = []AttributeParsingOverride{{ AttributeName: "objectGUID", - OverrideFunc: MicrosoftUUIDFromBinary, + OverrideFunc: MicrosoftUUIDFromBinary("objectGUID"), }} p.UserSearch.UIDAttribute = "objectGUID" }), @@ -549,7 +549,7 @@ func TestEndUserAuthentication(t *testing.T) { providerConfig: providerConfig(func(p *ProviderConfig) { p.UIDAttributeParsingOverrides = []AttributeParsingOverride{{ AttributeName: "objectGUID", - OverrideFunc: MicrosoftUUIDFromBinary, + OverrideFunc: MicrosoftUUIDFromBinary("objectGUID"), }} }), searchMocks: func(conn *mockldapconn.MockConn) { @@ -564,6 +564,89 @@ func TestEndUserAuthentication(t *testing.T) { }, wantAuthResponse: expectedAuthResponse(nil), }, + { + name: "override group parsing to create new group names", + username: testUpstreamUsername, + password: testUpstreamPassword, + providerConfig: providerConfig(func(p *ProviderConfig) { + p.GroupSearch.GroupNameAttribute = "sAMAccountName" + p.GroupAttributeParsingOverrides = []AttributeParsingOverride{{ + AttributeName: "sAMAccountName", + OverrideFunc: GroupSAMAccountNameWithDomainSuffix, + }} + }), + searchMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch(nil)).Return(exampleUserSearchResult, nil).Times(1) + conn.EXPECT().SearchWithPaging(expectedGroupSearch(func(r *ldap.SearchRequest) { + r.Attributes = []string{"sAMAccountName"} + }), expectedGroupSearchPageSize). + Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: "CN=Mammals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute("sAMAccountName", []string{"Mammals"}), + }, + }, + { + DN: "CN=Animals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute("sAMAccountName", []string{"Animals"}), + }, + }, + }, + Referrals: []string{}, // note that we are not following referrals at this time + Controls: []ldap.Control{}, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) + }, + wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { + r.Groups = []string{"Animals@activedirectory.mycompany.example.com", "Mammals@activedirectory.mycompany.example.com"} + }), + }, + { + name: "override group parsing when domain can't be determined from dn", + username: testUpstreamUsername, + password: testUpstreamPassword, + providerConfig: providerConfig(func(p *ProviderConfig) { + p.GroupSearch.GroupNameAttribute = "sAMAccountName" + p.GroupAttributeParsingOverrides = []AttributeParsingOverride{{ + AttributeName: "sAMAccountName", + OverrideFunc: GroupSAMAccountNameWithDomainSuffix, + }} + }), + searchMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch(nil)).Return(exampleUserSearchResult, nil).Times(1) + conn.EXPECT().SearchWithPaging(expectedGroupSearch(func(r *ldap.SearchRequest) { + r.Attributes = []string{"sAMAccountName"} + }), expectedGroupSearchPageSize). + Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: "no-domain-components", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute("sAMAccountName", []string{"Mammals"}), + }, + }, + { + DN: "CN=Animals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute("sAMAccountName", []string{"Animals"}), + }, + }, + }, + Referrals: []string{}, // note that we are not following referrals at this time + Controls: []ldap.Control{}, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantError: "error finding groups: did not find domain components in group dn: no-domain-components", + }, { name: "when dial fails", username: testUpstreamUsername, @@ -1378,7 +1461,7 @@ func TestGetMicrosoftFormattedUUID(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - actualUUIDString, err := MicrosoftUUIDFromBinary(tt.binaryUUID) + actualUUIDString, err := microsoftUUIDFromBinary(tt.binaryUUID) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) } else { @@ -1388,3 +1471,41 @@ func TestGetMicrosoftFormattedUUID(t *testing.T) { }) } } + +func TestGetDomainFromDistinguishedName(t *testing.T) { + tests := []struct { + name string + distinguishedName string + wantDomain string + wantErr string + }{ + { + name: "happy path", + distinguishedName: "CN=Mammals,OU=Users,OU=pinniped-ad,DC=activedirectory,DC=mycompany,DC=example,DC=com", + wantDomain: "activedirectory.mycompany.example.com", + }, + { + name: "lowercased happy path", + distinguishedName: "cn=Mammals,ou=Users,ou=pinniped-ad,dc=activedirectory,dc=mycompany,dc=example,dc=com", + wantDomain: "activedirectory.mycompany.example.com", + }, + { + name: "no domain components", + distinguishedName: "not-a-dn", + wantErr: "did not find domain components in group dn: not-a-dn", + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + actualDomain, err := getDomainFromDistinguishedName(tt.distinguishedName) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantDomain, actualDomain) + }) + } +} diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index ed2912a1..1ff73621 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -345,7 +345,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.TestUserIndirectGroupsSAMAccountNames, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames, }, { name: "activedirectory with custom options", maybeSkip: func(t *testing.T) { diff --git a/test/testlib/env.go b/test/testlib/env.go index 148086e2..7b8c71b8 100644 --- a/test/testlib/env.go +++ b/test/testlib/env.go @@ -82,27 +82,28 @@ 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"` - DefaultNamingContextSearchBase string `json:"defaultNamingContextSearchBase"` - 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"` - TestDeactivatedUserSAMAccountNameValue string `json:"TestDeactivatedUserSAMAccountNameValue"` - TestDeactivatedUserPassword string `json:"TestDeactivatedUserPassword"` + Host string `json:"host"` + StartTLSOnlyHost string `json:"startTLSOnlyHost"` + CABundle string `json:"caBundle"` + BindUsername string `json:"bindUsername"` + BindPassword string `json:"bindPassword"` + UserSearchBase string `json:"userSearchBase"` + DefaultNamingContextSearchBase string `json:"defaultNamingContextSearchBase"` + 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"` + TestUserIndirectGroupsSAMAccountPlusDomainNames []string `json:"TestUserIndirectGroupsSAMAccountPlusDomainNames"` + TestDeactivatedUserSAMAccountNameValue string `json:"TestDeactivatedUserSAMAccountNameValue"` + TestDeactivatedUserPassword string `json:"TestDeactivatedUserPassword"` } // ProxyEnv returns a set of environment variable strings (e.g., to combine with os.Environ()) which set up the configured test HTTP proxy. @@ -274,24 +275,25 @@ 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", ""), - TestUserMailAttributeValue: wantEnv("PINNIPED_TEST_AD_USER_EMAIL_ATTRIBUTE_VALUE", ""), - TestUserMailAttributeName: wantEnv("PINNIPED_TEST_AD_USER_EMAIL_ATTRIBUTE_NAME", ""), - 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", ""), ";")), - TestDeactivatedUserSAMAccountNameValue: wantEnv("PINNIPED_TEST_DEACTIVATED_AD_USER_SAMACCOUNTNAME", ""), - TestDeactivatedUserPassword: wantEnv("PINNIPED_TEST_DEACTIVATED_AD_USER_PASSWORD", ""), - DefaultNamingContextSearchBase: wantEnv("PINNIPED_TEST_AD_DEFAULTNAMINGCONTEXT_DN", ""), - UserSearchBase: wantEnv("PINNIPED_TEST_AD_USERS_DN", ""), - GroupSearchBase: wantEnv("PINNIPED_TEST_AD_USERS_DN", ""), + 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", ""), + TestUserMailAttributeValue: wantEnv("PINNIPED_TEST_AD_USER_EMAIL_ATTRIBUTE_VALUE", ""), + TestUserMailAttributeName: wantEnv("PINNIPED_TEST_AD_USER_EMAIL_ATTRIBUTE_NAME", ""), + 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", ""), ";")), + TestUserIndirectGroupsSAMAccountPlusDomainNames: filterEmpty(strings.Split(wantEnv("PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_SAMACCOUNTNAME_DOMAINNAMES", ""), ";")), + TestDeactivatedUserSAMAccountNameValue: wantEnv("PINNIPED_TEST_DEACTIVATED_AD_USER_SAMACCOUNTNAME", ""), + TestDeactivatedUserPassword: wantEnv("PINNIPED_TEST_DEACTIVATED_AD_USER_PASSWORD", ""), + DefaultNamingContextSearchBase: wantEnv("PINNIPED_TEST_AD_DEFAULTNAMINGCONTEXT_DN", ""), + UserSearchBase: wantEnv("PINNIPED_TEST_AD_USERS_DN", ""), + GroupSearchBase: wantEnv("PINNIPED_TEST_AD_USERS_DN", ""), } sort.Strings(result.SupervisorUpstreamLDAP.TestUserDirectGroupsCNs)