Add Configurable LDAP Max Page Size (hashicorp#19032)

* Add config flag for LDAP max page size * Add changelog * move changelog to correct file * cleanup * Default to non-paged searching for with -1 * Update website/content/api-docs/auth/ldap.mdx Co-authored-by: Austin Gebauer <[email protected]> * Update website/content/docs/auth/ldap.mdx Co-authored-by: Austin Gebauer <[email protected]> * Update tests --------- Co-authored-by: Austin Gebauer <[email protected]>
fopina · Apr 20, 2023 · 7f2deb1 · 7f2deb1
1 parent d51ae03
commit 7f2deb1
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 4 deletions.
diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go
@@ -1197,6 +1197,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) {
 			"token_period":           "5m",
 			"token_explicit_max_ttl": "24h",
 			"request_timeout":        cfg.RequestTimeout,
+			"max_page_size":          cfg.MaximumPageSize,
 			"connection_timeout":     cfg.ConnectionTimeout,
 		},
 		Storage:    storage,
@@ -1242,6 +1243,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) {
 			ConnectionTimeout:        cfg.ConnectionTimeout,
 			UsernameAsAlias:          false,
 			DerefAliases:             "never",
+			MaximumPageSize:          1000,
 		},
 	}
 

diff --git a/changelog/19032.txt b/changelog/19032.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+auth/ldap: Add max_page_size configurable to LDAP configuration
+```
diff --git a/helper/testhelpers/ldap/ldaphelper.go b/helper/testhelpers/ldap/ldaphelper.go
@@ -36,6 +36,7 @@ func PrepareTestContainer(t *testing.T, version string) (cleanup func(), cfg *ld
 	cfg.GroupDN = "ou=people,dc=planetexpress,dc=com"
 	cfg.GroupAttr = "cn"
 	cfg.RequestTimeout = 60
+	cfg.MaximumPageSize = 1000
 
 	svc, err := runner.StartService(context.Background(), func(ctx context.Context, host string, port int) (docker.ServiceConfig, error) {
 		connURL := fmt.Sprintf("ldap://%s:%d", host, port)

diff --git a/sdk/helper/ldaputil/client.go b/sdk/helper/ldaputil/client.go
@@ -432,7 +432,7 @@ func (c *Client) performLdapFilterGroupsSearchPaging(cfg *ConfigEntry, conn Pagi
 			cfg.GroupAttr,
 		},
 		SizeLimit: math.MaxInt32,
-	}, math.MaxInt32)
+	}, uint32(cfg.MaximumPageSize))
 	if err != nil {
 		return nil, fmt.Errorf("LDAP search failed: %w", err)
 	}
@@ -553,7 +553,7 @@ func (c *Client) GetLdapGroups(cfg *ConfigEntry, conn Connection, userDN string,
 	if cfg.UseTokenGroups {
 		entries, err = c.performLdapTokenGroupsSearch(cfg, conn, userDN)
 	} else {
-		if paging, ok := conn.(PagingConnection); ok {
+		if paging, ok := conn.(PagingConnection); ok && cfg.MaximumPageSize >= 0 {
 			entries, err = c.performLdapFilterGroupsSearchPaging(cfg, paging, userDN, username)
 		} else {
 			entries, err = c.performLdapFilterGroupsSearch(cfg, conn, userDN, username)

diff --git a/sdk/helper/ldaputil/config.go b/sdk/helper/ldaputil/config.go
@@ -9,6 +9,7 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
+	"math"
 	"strings"
 	"text/template"
 
@@ -251,6 +252,12 @@ Default: ({{.UserAttr}}={{.Username}})`,
 			Default:       "never",
 			AllowedValues: []interface{}{"never", "finding", "searching", "always"},
 		},
+
+		"max_page_size": {
+			Type:        framework.TypeInt,
+			Description: "The maximum number of results to return for a single paged query. If not set, the server default will be used for paged searches. A requested max_page_size of 0 is interpreted as no limit by LDAP servers. If set to a negative value, search requests will not be paged.",
+			Default:     math.MaxInt32,
+		},
 	}
 }
 
@@ -425,6 +432,10 @@ func NewConfigEntry(existing *ConfigEntry, d *framework.FieldData) (*ConfigEntry
 		cfg.DerefAliases = d.Get("dereference_aliases").(string)
 	}
 
+	if _, ok := d.Raw["max_page_size"]; ok || !hadExisting {
+		cfg.MaximumPageSize = d.Get("max_page_size").(int)
+	}
+
 	return cfg, nil
 }
 
@@ -453,6 +464,7 @@ type ConfigEntry struct {
 	RequestTimeout           int    `json:"request_timeout"`
 	ConnectionTimeout        int    `json:"connection_timeout"`
 	DerefAliases             string `json:"dereference_aliases"`
+	MaximumPageSize          int    `json:"max_page_size"`
 
 	// These json tags deviate from snake case because there was a past issue
 	// where the tag was being ignored, causing it to be jsonified as "CaseSensitiveNames", etc.
@@ -493,6 +505,7 @@ func (c *ConfigEntry) PasswordlessMap() map[string]interface{} {
 		"connection_timeout":     c.ConnectionTimeout,
 		"username_as_alias":      c.UsernameAsAlias,
 		"dereference_aliases":    c.DerefAliases,
+		"max_page_size":          c.MaximumPageSize,
 	}
 	if c.CaseSensitiveNames != nil {
 		m["case_sensitive_names"] = *c.CaseSensitiveNames

diff --git a/sdk/helper/ldaputil/config_test.go b/sdk/helper/ldaputil/config_test.go
@@ -175,6 +175,7 @@ var jsonConfigDefault = []byte(`
   "request_timeout": 90,
   "connection_timeout": 30,
   "dereference_aliases": "never",
+  "max_page_size": 2147483647,
   "CaseSensitiveNames": false,
   "ClientTLSCert": "",
   "ClientTLSKey": ""

diff --git a/website/content/api-docs/auth/ldap.mdx b/website/content/api-docs/auth/ldap.mdx
@@ -97,6 +97,12 @@ This endpoint configures the LDAP auth method.
 - `dereference_aliases` `(string: never)` - When aliases should be dereferenced
   on search operations. Accepted values are 'never', 'finding', 'searching',
   'always'. Defaults to 'never'.
+- `max_page_size` `(int: math.MaxInt32)` - If set to a value greater than 0, the LDAP
+  backend will use the LDAP server's paged search control to request pages of
+  up to the given size. This can be used to avoid hitting the LDAP server's
+  maximum result size limit. A value of 0 will be interpreted by the LDAP
+  server as unlimited. If set to -1, the LDAP backend will not use the
+  paged search control.
 
 @include 'tokenfields.mdx'
 
@@ -129,7 +135,8 @@ $ curl \
   "url": "ldaps://ldap.myorg.com:636",
   "username_as_alias": false,
   "userattr": "samaccountname",
-  "userdn": "ou=Users,dc=example,dc=com"
+  "userdn": "ou=Users,dc=example,dc=com",
+  "max_page_size": 1000
 }
 ```
 

diff --git a/website/content/docs/auth/ldap.mdx b/website/content/docs/auth/ldap.mdx
@@ -156,7 +156,7 @@ Use `vault path-help` for more details.
 ### Other
 
 - `username_as_alias` (bool, optional) - If set to true, forces the auth method to use the username passed by the user as the alias name.
-
+- `max_page_size` (int, optional) - The maximum number of results to return for a single LDAP query. This is useful for preventing large queries from being run against the LDAP server. The default is the maximum value for an int32.
 
 ## Examples: