Make new combined sAMAccountName@domain attribute the group name

Also change default username attribute to userPrincipalName

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"
+	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)

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))",
+						Filter:            "(&(objectClass=person)(!(objectClass=computer))(!(showInAdvancedViewOnly=TRUE))(|(sAMAccountName={})(mail={})(userPrincipalName={}))(sAMAccountType=805306368))",
-						UsernameAttribute: "sAMAccountName",
+						UsernameAttribute: "userPrincipalName",
 						UIDAttribute:      "objectGUID",
 					},
 					GroupSearch: upstreamldap.GroupSearchConfig{
@@ -1191,6 +1191,58 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						GroupNameAttribute: "sAMAccountName",
 					},
 					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.Equal(t, len(expectedUIDAttributeParsingOverrides), len(actualUIDAttributeParsingOverrides))
-				require.Len(t, expectedUIDAttributeParsingOverrides, 1)
+				for i := range expectedUIDAttributeParsingOverrides {
-				require.Equal(t, expectedUIDAttributeParsingOverrides[0].AttributeName, actualUIDAttributeParsingOverrides[0].AttributeName)
+					require.Equal(t, expectedUIDAttributeParsingOverrides[i].AttributeName, actualUIDAttributeParsingOverrides[i].AttributeName)
-				require.Equal(t, reflect.ValueOf(expectedUIDAttributeParsingOverrides[0].OverrideFunc).Pointer(), reflect.ValueOf(actualUIDAttributeParsingOverrides[0].OverrideFunc).Pointer())
+					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)
 			}

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
+}

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)
+		})
+	}
+}

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) {

test/testlib/env.go

@@ -101,6 +101,7 @@ type TestLDAPUpstream struct {
 	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"`
 }
@@ -287,6 +288,7 @@ func loadEnvVars(t *testing.T, result *TestEnv) {
 		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", ""),