Merge pull request #649 from vmware-tanzu/change_ldap_groupname_default

Default `groupSearch.attributes.groupName` to "dn" instead of "cn" in LDAPIdentityProvider spec

apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go.tmpl

@@ -68,8 +68,8 @@ type LDAPIdentityProviderGroupSearchAttributes struct {
 	// GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name
 	// in the user's list of groups after a successful authentication.
 	// The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP
-	// server in the user's entry. Distinguished names can be used by specifying lower-case "dn".
+	// server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn".
-	// Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+	// Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 	// +optional
 	GroupName string `json:"groupName,omitempty"`
 }

deploy/supervisor/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml

@@ -86,10 +86,10 @@ spec:
                           in the user's list of groups after a successful authentication.
                           The value of this field is case-sensitive and must match
                           the case of the attribute name returned by the LDAP server
-                          in the user's entry. Distinguished names can be used by
+                          in the user's entry. E.g. "cn" for common name. Distinguished
-                          specifying lower-case "dn". Optional. When not specified,
+                          names can be used by specifying lower-case "dn". Optional.
-                          the default will act as if the GroupName were specified
+                          When not specified, the default will act as if the GroupName
-                          as "cn" (common name).
+                          were specified as "dn" (distinguished name).
                         type: string
                     type: object
                   base:

generated/1.17/README.adoc

@@ -852,7 +852,7 @@ LDAPIdentityProvider describes the configuration of an upstream Lightweight Dire
 [cols="25a,75a", options="header"]
 |===
 | Field | Description
-| *`groupName`* __string__ | GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name in the user's list of groups after a successful authentication. The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP server in the user's entry. Distinguished names can be used by specifying lower-case "dn". Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+| *`groupName`* __string__ | GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name in the user's list of groups after a successful authentication. The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn". Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 |===
 
 

generated/1.17/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go

@@ -68,8 +68,8 @@ type LDAPIdentityProviderGroupSearchAttributes struct {
 	// GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name
 	// in the user's list of groups after a successful authentication.
 	// The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP
-	// server in the user's entry. Distinguished names can be used by specifying lower-case "dn".
+	// server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn".
-	// Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+	// Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 	// +optional
 	GroupName string `json:"groupName,omitempty"`
 }

generated/1.17/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml

@@ -86,10 +86,10 @@ spec:
                           in the user's list of groups after a successful authentication.
                           The value of this field is case-sensitive and must match
                           the case of the attribute name returned by the LDAP server
-                          in the user's entry. Distinguished names can be used by
+                          in the user's entry. E.g. "cn" for common name. Distinguished
-                          specifying lower-case "dn". Optional. When not specified,
+                          names can be used by specifying lower-case "dn". Optional.
-                          the default will act as if the GroupName were specified
+                          When not specified, the default will act as if the GroupName
-                          as "cn" (common name).
+                          were specified as "dn" (distinguished name).
                         type: string
                     type: object
                   base:

generated/1.18/README.adoc

@@ -852,7 +852,7 @@ LDAPIdentityProvider describes the configuration of an upstream Lightweight Dire
 [cols="25a,75a", options="header"]
 |===
 | Field | Description
-| *`groupName`* __string__ | GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name in the user's list of groups after a successful authentication. The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP server in the user's entry. Distinguished names can be used by specifying lower-case "dn". Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+| *`groupName`* __string__ | GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name in the user's list of groups after a successful authentication. The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn". Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 |===
 
 

generated/1.18/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go

@@ -68,8 +68,8 @@ type LDAPIdentityProviderGroupSearchAttributes struct {
 	// GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name
 	// in the user's list of groups after a successful authentication.
 	// The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP
-	// server in the user's entry. Distinguished names can be used by specifying lower-case "dn".
+	// server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn".
-	// Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+	// Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 	// +optional
 	GroupName string `json:"groupName,omitempty"`
 }

generated/1.18/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml

@@ -86,10 +86,10 @@ spec:
                           in the user's list of groups after a successful authentication.
                           The value of this field is case-sensitive and must match
                           the case of the attribute name returned by the LDAP server
-                          in the user's entry. Distinguished names can be used by
+                          in the user's entry. E.g. "cn" for common name. Distinguished
-                          specifying lower-case "dn". Optional. When not specified,
+                          names can be used by specifying lower-case "dn". Optional.
-                          the default will act as if the GroupName were specified
+                          When not specified, the default will act as if the GroupName
-                          as "cn" (common name).
+                          were specified as "dn" (distinguished name).
                         type: string
                     type: object
                   base:

generated/1.19/README.adoc

@@ -852,7 +852,7 @@ LDAPIdentityProvider describes the configuration of an upstream Lightweight Dire
 [cols="25a,75a", options="header"]
 |===
 | Field | Description
-| *`groupName`* __string__ | GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name in the user's list of groups after a successful authentication. The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP server in the user's entry. Distinguished names can be used by specifying lower-case "dn". Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+| *`groupName`* __string__ | GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name in the user's list of groups after a successful authentication. The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn". Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 |===
 
 

generated/1.19/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go

@@ -68,8 +68,8 @@ type LDAPIdentityProviderGroupSearchAttributes struct {
 	// GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name
 	// in the user's list of groups after a successful authentication.
 	// The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP
-	// server in the user's entry. Distinguished names can be used by specifying lower-case "dn".
+	// server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn".
-	// Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+	// Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 	// +optional
 	GroupName string `json:"groupName,omitempty"`
 }

generated/1.19/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml

@@ -86,10 +86,10 @@ spec:
                           in the user's list of groups after a successful authentication.
                           The value of this field is case-sensitive and must match
                           the case of the attribute name returned by the LDAP server
-                          in the user's entry. Distinguished names can be used by
+                          in the user's entry. E.g. "cn" for common name. Distinguished
-                          specifying lower-case "dn". Optional. When not specified,
+                          names can be used by specifying lower-case "dn". Optional.
-                          the default will act as if the GroupName were specified
+                          When not specified, the default will act as if the GroupName
-                          as "cn" (common name).
+                          were specified as "dn" (distinguished name).
                         type: string
                     type: object
                   base:

generated/1.20/README.adoc

@@ -852,7 +852,7 @@ LDAPIdentityProvider describes the configuration of an upstream Lightweight Dire
 [cols="25a,75a", options="header"]
 |===
 | Field | Description
-| *`groupName`* __string__ | GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name in the user's list of groups after a successful authentication. The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP server in the user's entry. Distinguished names can be used by specifying lower-case "dn". Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+| *`groupName`* __string__ | GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name in the user's list of groups after a successful authentication. The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn". Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 |===
 
 

generated/1.20/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go

@@ -68,8 +68,8 @@ type LDAPIdentityProviderGroupSearchAttributes struct {
 	// GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name
 	// in the user's list of groups after a successful authentication.
 	// The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP
-	// server in the user's entry. Distinguished names can be used by specifying lower-case "dn".
+	// server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn".
-	// Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+	// Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 	// +optional
 	GroupName string `json:"groupName,omitempty"`
 }

generated/1.20/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml

@@ -86,10 +86,10 @@ spec:
                           in the user's list of groups after a successful authentication.
                           The value of this field is case-sensitive and must match
                           the case of the attribute name returned by the LDAP server
-                          in the user's entry. Distinguished names can be used by
+                          in the user's entry. E.g. "cn" for common name. Distinguished
-                          specifying lower-case "dn". Optional. When not specified,
+                          names can be used by specifying lower-case "dn". Optional.
-                          the default will act as if the GroupName were specified
+                          When not specified, the default will act as if the GroupName
-                          as "cn" (common name).
+                          were specified as "dn" (distinguished name).
                         type: string
                     type: object
                   base:

generated/latest/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go

@@ -68,8 +68,8 @@ type LDAPIdentityProviderGroupSearchAttributes struct {
 	// GroupName specifies the name of the attribute in the LDAP entries whose value shall become a group name
 	// in the user's list of groups after a successful authentication.
 	// The value of this field is case-sensitive and must match the case of the attribute name returned by the LDAP
-	// server in the user's entry. Distinguished names can be used by specifying lower-case "dn".
+	// server in the user's entry. E.g. "cn" for common name. Distinguished names can be used by specifying lower-case "dn".
-	// Optional. When not specified, the default will act as if the GroupName were specified as "cn" (common name).
+	// Optional. When not specified, the default will act as if the GroupName were specified as "dn" (distinguished name).
 	// +optional
 	GroupName string `json:"groupName,omitempty"`
 }

internal/upstreamldap/upstreamldap.go

@@ -31,7 +31,6 @@ import (
 const (
 	ldapsScheme                             = "ldaps"
 	distinguishedNameAttributeName          = "dn"
-	commonNameAttributeName                 = "cn"
 	searchFilterInterpolationLocationMarker = "{}"
 	groupSearchPageSize                     = uint32(250)
 	defaultLDAPPort                         = uint16(389)
@@ -367,7 +366,7 @@ func (p *Provider) searchGroupsForUserDN(conn Conn, userDN string) ([]string, er
 
 	groupAttributeName := p.c.GroupSearch.GroupNameAttribute
 	if len(groupAttributeName) == 0 {
-		groupAttributeName = commonNameAttributeName
+		groupAttributeName = distinguishedNameAttributeName
 	}
 
 	groups := []string{}
@@ -493,7 +492,7 @@ func (p *Provider) userSearchRequestedAttributes() []string {
 func (p *Provider) groupSearchRequestedAttributes() []string {
 	switch p.c.GroupSearch.GroupNameAttribute {
 	case "":
-		return []string{commonNameAttributeName}
+		return []string{}
 	case distinguishedNameAttributeName:
 		return []string{}
 	default:

internal/upstreamldap/upstreamldap_test.go

@@ -315,6 +315,29 @@ func TestEndUserAuthentication(t *testing.T) {
 				r.UID = base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultDNValue))
 			}),
 		},
+		{
+			name:     "when the GroupNameAttribute is empty then it defaults to dn",
+			username: testUpstreamUsername,
+			password: testUpstreamPassword,
+			providerConfig: providerConfig(func(p *ProviderConfig) {
+				p.GroupSearch.GroupNameAttribute = "" // blank means to use dn
+			}),
+			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{}
+				}), expectedGroupSearchPageSize).
+					Return(exampleGroupSearchResult, 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{testGroupSearchResultDNValue1, testGroupSearchResultDNValue2}
+			}),
+		},
 		{
 			name:     "when the GroupNameAttribute is dn",
 			username: testUpstreamUsername,
@@ -339,11 +362,11 @@ func TestEndUserAuthentication(t *testing.T) {
 			}),
 		},
 		{
-			name:     "when the GroupNameAttribute is empty then it defaults to cn",
+			name:     "when the GroupNameAttribute is cn",
 			username: testUpstreamUsername,
 			password: testUpstreamPassword,
 			providerConfig: providerConfig(func(p *ProviderConfig) {
-				p.GroupSearch.GroupNameAttribute = "" // blank means to use cn
+				p.GroupSearch.GroupNameAttribute = "cn"
 			}),
 			searchMocks: func(conn *mockldapconn.MockConn) {
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)

test/integration/e2e_test.go

@@ -278,7 +278,7 @@ func TestE2EFullIntegration(t *testing.T) {
 	// Add an LDAP upstream IDP and try using it to authenticate during kubectl commands.
 	t.Run("with Supervisor LDAP upstream IDP", func(t *testing.T) {
 		expectedUsername := env.SupervisorUpstreamLDAP.TestUserMailAttributeValue
-		expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsCNs
+		expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs
 
 		// Create a ClusterRoleBinding to give our test user from the upstream read-only access to the cluster.
 		library.CreateTestClusterRoleBinding(t,
@@ -321,7 +321,7 @@ func TestE2EFullIntegration(t *testing.T) {
 				Base:   env.SupervisorUpstreamLDAP.GroupSearchBase,
 				Filter: "", // use the default value of "member={}"
 				Attributes: idpv1alpha1.LDAPIdentityProviderGroupSearchAttributes{
-					GroupName: "", // use the default value of "cn"
+					GroupName: "", // use the default value of "dn"
 				},
 			},
 		}, idpv1alpha1.LDAPPhaseReady)

test/integration/ldap_client_test.go

@@ -243,6 +243,20 @@ func TestLDAPSearch(t *testing.T) {
 				}},
 			},
 		},
+		{
+			name:     "using the default group name attribute, which is dn",
+			username: "pinny",
+			password: pinnyPassword,
+			provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) {
+				p.GroupSearch.GroupNameAttribute = ""
+			})),
+			wantAuthResponse: &authenticator.Response{
+				User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{
+					"cn=ball-game-players,ou=beach-groups,ou=groups,dc=pinniped,dc=dev",
+					"cn=seals,ou=groups,dc=pinniped,dc=dev",
+				}},
+			},
+		},
 		{
 			name:     "using some other custom group name attribute",
 			username: "pinny",
@@ -675,8 +689,8 @@ func defaultProviderConfig(env *library.TestEnv, port string) *upstreamldap.Prov
 		},
 		GroupSearch: upstreamldap.GroupSearchConfig{
 			Base:               "ou=groups,dc=pinniped,dc=dev",
-			Filter:             "", // defaults to member={}
+			Filter:             "",   // defaults to member={}
-			GroupNameAttribute: "", // defaults to cn
+			GroupNameAttribute: "cn", // defaults to dn, but here we set it to cn
 		},
 	}
 }