Skip to content

Commit

Permalink
sdk/ldaputil: add connection_timeout configurable (#20144)
Browse files Browse the repository at this point in the history
* sdk/ldaputil: add connection_timeout configurable

* changelog

* Update doc

* Fix test

* Change default to 30s
  • Loading branch information
jasonodonnell authored Apr 13, 2023
1 parent a7c9559 commit 2f7f0d2
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 9 deletions.
7 changes: 7 additions & 0 deletions builtin/credential/ldap/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,7 @@ func testAccStepConfigUrl(t *testing.T, cfg *ldaputil.ConfigEntry) logicaltest.T
"case_sensitive_names": true,
"token_policies": "abc,xyz",
"request_timeout": cfg.RequestTimeout,
"connection_timeout": cfg.ConnectionTimeout,
"username_as_alias": cfg.UsernameAsAlias,
},
}
Expand All @@ -854,6 +855,7 @@ func testAccStepConfigUrlWithAuthBind(t *testing.T, cfg *ldaputil.ConfigEntry) l
"case_sensitive_names": true,
"token_policies": "abc,xyz",
"request_timeout": cfg.RequestTimeout,
"connection_timeout": cfg.ConnectionTimeout,
},
}
}
Expand All @@ -874,6 +876,7 @@ func testAccStepConfigUrlWithDiscover(t *testing.T, cfg *ldaputil.ConfigEntry) l
"case_sensitive_names": true,
"token_policies": "abc,xyz",
"request_timeout": cfg.RequestTimeout,
"connection_timeout": cfg.ConnectionTimeout,
},
}
}
Expand All @@ -891,6 +894,7 @@ func testAccStepConfigUrlNoGroupDN(t *testing.T, cfg *ldaputil.ConfigEntry) logi
"discoverdn": true,
"case_sensitive_names": true,
"request_timeout": cfg.RequestTimeout,
"connection_timeout": cfg.ConnectionTimeout,
},
}
}
Expand All @@ -911,6 +915,7 @@ func testAccStepConfigUrlWarningCheck(t *testing.T, cfg *ldaputil.ConfigEntry, o
"case_sensitive_names": true,
"token_policies": "abc,xyz",
"request_timeout": cfg.RequestTimeout,
"connection_timeout": cfg.ConnectionTimeout,
},
Check: func(response *logical.Response) error {
if len(response.Warnings) == 0 {
Expand Down Expand Up @@ -1192,6 +1197,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) {
"token_period": "5m",
"token_explicit_max_ttl": "24h",
"request_timeout": cfg.RequestTimeout,
"connection_timeout": cfg.ConnectionTimeout,
},
Storage: storage,
Connection: &logical.Connection{},
Expand Down Expand Up @@ -1233,6 +1239,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) {
CaseSensitiveNames: falseBool,
UsePre111GroupCNBehavior: new(bool),
RequestTimeout: cfg.RequestTimeout,
ConnectionTimeout: cfg.ConnectionTimeout,
UsernameAsAlias: false,
DerefAliases: "never",
},
Expand Down
4 changes: 4 additions & 0 deletions changelog/20144.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
```release-note:improvement
sdk/ldaputil: added `connection_timeout` to tune connection timeout duration
for all LDAP plugins.
```
6 changes: 6 additions & 0 deletions sdk/helper/ldaputil/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ func (c *Client) DialLDAP(cfg *ConfigEntry) (Connection, error) {
var retErr *multierror.Error
var conn Connection
urls := strings.Split(cfg.Url, ",")

// Default timeout in the pacakge is 60 seconds, which we default to on our
// end. This is useful if you want to take advantage of the URL list to increase
// availability of LDAP.
ldap.DefaultTimeout = time.Duration(cfg.ConnectionTimeout) * time.Second

for _, uut := range urls {
u, err := url.Parse(uut)
if err != nil {
Expand Down
12 changes: 12 additions & 0 deletions sdk/helper/ldaputil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,12 @@ Default: ({{.UserAttr}}={{.Username}})`,
Default: "90s",
},

"connection_timeout": {
Type: framework.TypeDurationSecond,
Description: "Timeout, in seconds, when attempting to connect to the LDAP server before trying the next URL in the configuration.",
Default: "30s",
},

"dereference_aliases": {
Type: framework.TypeString,
Description: "When aliases should be dereferenced on search operations. Accepted values are 'never', 'finding', 'searching', 'always'. Defaults to 'never'.",
Expand Down Expand Up @@ -411,6 +417,10 @@ func NewConfigEntry(existing *ConfigEntry, d *framework.FieldData) (*ConfigEntry
cfg.RequestTimeout = d.Get("request_timeout").(int)
}

if _, ok := d.Raw["connection_timeout"]; ok || !hadExisting {
cfg.ConnectionTimeout = d.Get("connection_timeout").(int)
}

if _, ok := d.Raw["dereference_aliases"]; ok || !hadExisting {
cfg.DerefAliases = d.Get("dereference_aliases").(string)
}
Expand Down Expand Up @@ -441,6 +451,7 @@ type ConfigEntry struct {
UseTokenGroups bool `json:"use_token_groups"`
UsePre111GroupCNBehavior *bool `json:"use_pre111_group_cn_behavior"`
RequestTimeout int `json:"request_timeout"`
ConnectionTimeout int `json:"connection_timeout"`
DerefAliases string `json:"dereference_aliases"`

// These json tags deviate from snake case because there was a past issue
Expand Down Expand Up @@ -479,6 +490,7 @@ func (c *ConfigEntry) PasswordlessMap() map[string]interface{} {
"use_token_groups": c.UseTokenGroups,
"anonymous_group_search": c.AnonymousGroupSearch,
"request_timeout": c.RequestTimeout,
"connection_timeout": c.ConnectionTimeout,
"username_as_alias": c.UsernameAsAlias,
"dereference_aliases": c.DerefAliases,
}
Expand Down
21 changes: 12 additions & 9 deletions sdk/helper/ldaputil/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,16 @@ func testConfig(t *testing.T) *ConfigEntry {
t.Helper()

return &ConfigEntry{
Url: "ldap://138.91.247.105",
UserDN: "example,com",
BindDN: "kitty",
BindPassword: "cats",
TLSMaxVersion: "tls12",
TLSMinVersion: "tls12",
RequestTimeout: 30,
ClientTLSCert: "",
ClientTLSKey: "",
Url: "ldap://138.91.247.105",
UserDN: "example,com",
BindDN: "kitty",
BindPassword: "cats",
TLSMaxVersion: "tls12",
TLSMinVersion: "tls12",
RequestTimeout: 30,
ConnectionTimeout: 15,
ClientTLSCert: "",
ClientTLSKey: "",
}
}

Expand Down Expand Up @@ -141,6 +142,7 @@ var jsonConfig = []byte(`{
"tls_max_version": "tls12",
"tls_min_version": "tls12",
"request_timeout": 30,
"connection_timeout": 15,
"ClientTLSCert": "",
"ClientTLSKey": ""
}`)
Expand Down Expand Up @@ -171,6 +173,7 @@ var jsonConfigDefault = []byte(`
"use_pre111_group_cn_behavior": null,
"username_as_alias": false,
"request_timeout": 90,
"connection_timeout": 30,
"dereference_aliases": "never",
"CaseSensitiveNames": false,
"ClientTLSCert": "",
Expand Down
3 changes: 3 additions & 0 deletions website/content/api-docs/auth/ldap.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ This endpoint configures the LDAP auth method.
names will be normalized to lower case. Case will still be preserved when
sending the username to the LDAP server at login time; this is only for
matching local user/group definitions.
- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds,
when attempting to connect to the LDAP server before trying the next URL in
the configuration.
- `request_timeout` `(integer: 90 or string: "90s")` - Timeout, in seconds, for
the connection when making requests against the server before returning back
an error.
Expand Down
1 change: 1 addition & 0 deletions website/content/api-docs/secret/ad.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ text that fulfills those requirements. `{{PASSWORD}}` must appear exactly once a
### Connection parameters

- `url` (string, optional) - The LDAP server to connect to. Examples: `ldaps://ldap.myorg.com`, `ldaps://ldap.myorg.com:636`. This can also be a comma-delineated list of URLs, e.g. `ldaps://ldap.myorg.com,ldaps://ldap.myorg.com:636`, in which case the servers will be tried in-order if there are errors during the connection process. Default is `ldap://127.0.0.1`.
- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds, when attempting to connect to the LDAP server before trying the next URL in the configuration.
- `request_timeout` `(integer: 90 or string: "90s")` - Timeout, in seconds, for the connection when making requests against the server before returning back an error.
- `starttls` (bool, optional) - If true, issues a `StartTLS` command after establishing an unencrypted connection.
- `insecure_tls` - (bool, optional) - If true, skips LDAP server SSL certificate verification - insecure, use with caution!
Expand Down
3 changes: 3 additions & 0 deletions website/content/api-docs/secret/ldap.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ to search and change entry passwords in LDAP.
string for authentication. The constructed UPN will appear as `[binddn]@[upndomain]`. For
example, if `upndomain=example.com` and `binddn=admin`, the UPN string `admin@example.com`
will be used to log in to Active Directory.
- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds,
when attempting to connect to the LDAP server before trying the next URL in
the configuration.
- `request_timeout` `(integer: 90, string: "90s" <optional>)` - Timeout, in seconds, for the connection when
making requests against the server before returning back an error.
- `starttls` `(bool: <optional>)` - If true, issues a `StartTLS` command after establishing an unencrypted connection.
Expand Down

0 comments on commit 2f7f0d2

Please sign in to comment.