tighten ldap sync query types

openshift · Oct 12, 2015 · aa31a91 · aa31a91
1 parent 986ffa9
commit aa31a91
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 29 deletions.
diff --git a/pkg/auth/ldaputil/query.go b/pkg/auth/ldaputil/query.go
@@ -96,27 +96,37 @@ type LDAPQueryOnAttribute struct {
 	QueryAttribute string
 }
 
-// NewLDAPQueryOnAttribute converts a user-provided LDAPQuery into a version we can use by parsing
-// the input and combining it with a set of name attributes
-func NewLDAPQueryOnAttribute(config api.LDAPQuery, attribute string) (LDAPQueryOnAttribute, error) {
+// NewLDAPQuery converts a user-provided LDAPQuery into a version we can use
+func NewLDAPQuery(config api.LDAPQuery) (LDAPQuery, error) {
 	scope, err := DetermineLDAPScope(config.Scope)
 	if err != nil {
-		return LDAPQueryOnAttribute{}, err
+		return LDAPQuery{}, err
 	}
 
 	derefAliases, err := DetermineDerefAliasesBehavior(config.DerefAliases)
+	if err != nil {
+		return LDAPQuery{}, err
+	}
+
+	return LDAPQuery{
+		BaseDN:       config.BaseDN,
+		Scope:        scope,
+		DerefAliases: derefAliases,
+		TimeLimit:    config.TimeLimit,
+		Filter:       config.Filter,
+	}, nil
+}
+
+// NewLDAPQueryOnAttribute converts a user-provided LDAPQuery into a version we can use by parsing
+// the input and combining it with a set of name attributes
+func NewLDAPQueryOnAttribute(config api.LDAPQuery, attribute string) (LDAPQueryOnAttribute, error) {
+	ldapQuery, err := NewLDAPQuery(config)
 	if err != nil {
 		return LDAPQueryOnAttribute{}, err
 	}
 
 	return LDAPQueryOnAttribute{
-		LDAPQuery: LDAPQuery{
-			BaseDN:       config.BaseDN,
-			Scope:        scope,
-			DerefAliases: derefAliases,
-			TimeLimit:    config.TimeLimit,
-			Filter:       config.Filter,
-		},
+		LDAPQuery:      ldapQuery,
 		QueryAttribute: attribute,
 	}, nil
 }

diff --git a/pkg/cmd/experimental/syncgroups/ad/augmented_ldapinterface.go b/pkg/cmd/experimental/syncgroups/ad/augmented_ldapinterface.go
@@ -11,7 +11,7 @@ import (
 
 // NewLDAPInterface builds a new LDAPInterface using a schema-appropriate config
 func NewAugmentedADLDAPInterface(clientConfig *ldaputil.LDAPClientConfig,
-	userQuery ldaputil.LDAPQueryOnAttribute,
+	userQuery ldaputil.LDAPQuery,
 	groupMembershipAttributes []string,
 	userNameAttributes []string,
 	groupQuery ldaputil.LDAPQueryOnAttribute,

diff --git a/pkg/cmd/experimental/syncgroups/ad/ldapinterface.go b/pkg/cmd/experimental/syncgroups/ad/ldapinterface.go
@@ -13,7 +13,7 @@ import (
 
 // NewADLDAPInterface builds a new ADLDAPInterface using a schema-appropriate config
 func NewADLDAPInterface(clientConfig *ldaputil.LDAPClientConfig,
-	userQuery ldaputil.LDAPQueryOnAttribute,
+	userQuery ldaputil.LDAPQuery,
 	groupMembershipAttributes []string,
 	userNameAttributes []string) *ADLDAPInterface {
 
@@ -35,9 +35,8 @@ type ADLDAPInterface struct {
 	// clientConfig holds LDAP connection information
 	clientConfig *ldaputil.LDAPClientConfig
 
-	// userQuery holds the information necessary to make an LDAP query for a specific
-	// first-class user entry on the LDAP server
-	userQuery ldaputil.LDAPQueryOnAttribute
+	// userQuery holds the information necessary to make an LDAP query for all first-class user entries on the LDAP server
+	userQuery ldaputil.LDAPQuery
 	// groupMembershipAttributes defines which attributes on an LDAP user entry will be interpreted as its ldapGroupUID
 	groupMembershipAttributes []string
 	// UserNameAttributes defines which attributes on an LDAP user entry will be interpreted as its name
@@ -62,10 +61,9 @@ func (e *ADLDAPInterface) ExtractMembers(ldapGroupUID string) ([]*ldap.Entry, er
 
 	// check for all users with ldapGroupUID in any of the allowed member attributes
 	for _, currAttribute := range e.groupMembershipAttributes {
-		currQuery := e.userQuery
-		currQuery.QueryAttribute = currAttribute
+		currQuery := ldaputil.LDAPQueryOnAttribute{LDAPQuery: e.userQuery, QueryAttribute: currAttribute}
 
-		searchRequest, err := currQuery.NewSearchRequest(ldapGroupUID, e.requiredUserAttributes())
+		searchRequest, err := currQuery.NewSearchRequest(ldapGroupUID, e.groupMembershipAttributes)
 		if err != nil {
 			return nil, err
 		}
@@ -106,7 +104,7 @@ func (e *ADLDAPInterface) populateCache() error {
 		return nil
 	}
 
-	searchRequest := e.userQuery.LDAPQuery.NewSearchRequest(e.requiredUserAttributes())
+	searchRequest := e.userQuery.NewSearchRequest(e.groupMembershipAttributes)
 
 	userEntries, err := ldaputil.QueryForEntries(e.clientConfig, searchRequest)
 	if err != nil {
@@ -145,10 +143,3 @@ func isEntryPresent(haystack []*ldap.Entry, needle *ldap.Entry) bool {
 
 	return false
 }
-
-func (e *ADLDAPInterface) requiredUserAttributes() []string {
-	attributes := sets.NewString(e.groupMembershipAttributes...)
-	attributes.Insert(e.userNameAttributes...)
-
-	return attributes.List()
-}
diff --git a/pkg/cmd/experimental/syncgroups/cli/ad.go b/pkg/cmd/experimental/syncgroups/cli/ad.go
@@ -38,7 +38,7 @@ func (b *ADSyncBuilder) getADLDAPInterface() (*ad.ADLDAPInterface, error) {
 		return b.adLDAPInterface, nil
 	}
 
-	userQuery, err := ldaputil.NewLDAPQueryOnAttribute(b.Config.AllUsersQuery, "dn")
+	userQuery, err := ldaputil.NewLDAPQuery(b.Config.AllUsersQuery)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/cmd/experimental/syncgroups/cli/augmented_ad.go b/pkg/cmd/experimental/syncgroups/cli/augmented_ad.go
@@ -46,7 +46,7 @@ func (b *AugmentedADSyncBuilder) getAugmentedADLDAPInterface() (*ad.AugmentedADL
 		return b.augmentedADLDAPInterface, nil
 	}
 
-	userQuery, err := ldaputil.NewLDAPQueryOnAttribute(b.Config.AllUsersQuery, "dn")
+	userQuery, err := ldaputil.NewLDAPQuery(b.Config.AllUsersQuery)
 	if err != nil {
 		return nil, err
 	}