auth/ldap: add resp warning if userfilter doesn't consider userattr (#14095)

* auth/ldap: add resp warning if userfilter doesn't consider userattr

* add changelog entry

builtin/credential/ldap/backend_test.go

@@ -11,6 +11,7 @@ import (
 	goldap "github.com/go-ldap/ldap/v3"
 	"github.com/go-test/deep"
 	hclog "github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/go-secure-stdlib/strutil"
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/helper/testhelpers/ldap"
 	logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical"
@@ -505,6 +506,30 @@ func TestBackend_basic_authbind_userfilter(t *testing.T) {
 	cleanup, cfg := ldap.PrepareTestContainer(t, "latest")
 	defer cleanup()
 
+	// userattr not used in the userfilter should result in a warning in the response
+	cfg.UserFilter = "((mail={{.Username}}))"
+	logicaltest.Test(t, logicaltest.TestCase{
+		CredentialBackend: b,
+		Steps: []logicaltest.TestStep{
+			testAccStepConfigUrlWarningCheck(t, cfg, logical.UpdateOperation, []string{userFilterWarning}),
+			testAccStepConfigUrlWarningCheck(t, cfg, logical.ReadOperation, []string{userFilterWarning}),
+		},
+	})
+
+	// If both upndomain and userfilter is set, ensure that a warning is still
+	// returned if userattr is not considered
+	cfg.UPNDomain = "planetexpress.com"
+
+	logicaltest.Test(t, logicaltest.TestCase{
+		CredentialBackend: b,
+		Steps: []logicaltest.TestStep{
+			testAccStepConfigUrlWarningCheck(t, cfg, logical.UpdateOperation, []string{userFilterWarning}),
+			testAccStepConfigUrlWarningCheck(t, cfg, logical.ReadOperation, []string{userFilterWarning}),
+		},
+	})
+
+	cfg.UPNDomain = ""
+
 	// Add a liberal user filter, allowing to log in with either cn or email
 	cfg.UserFilter = "(|({{.UserAttr}}={{.Username}})(mail={{.Username}}))"
 
@@ -842,6 +867,36 @@ func testAccStepConfigUrlNoGroupDN(t *testing.T, cfg *ldaputil.ConfigEntry) logi
 	}
 }
 
+func testAccStepConfigUrlWarningCheck(t *testing.T, cfg *ldaputil.ConfigEntry, operation logical.Operation, warnings []string) logicaltest.TestStep {
+	return logicaltest.TestStep{
+		Operation: operation,
+		Path:      "config",
+		Data: map[string]interface{}{
+			"url":                  cfg.Url,
+			"userattr":             cfg.UserAttr,
+			"userdn":               cfg.UserDN,
+			"userfilter":           cfg.UserFilter,
+			"groupdn":              cfg.GroupDN,
+			"groupattr":            cfg.GroupAttr,
+			"binddn":               cfg.BindDN,
+			"bindpass":             cfg.BindPassword,
+			"case_sensitive_names": true,
+			"token_policies":       "abc,xyz",
+			"request_timeout":      cfg.RequestTimeout,
+		},
+		Check: func(response *logical.Response) error {
+			if len(response.Warnings) == 0 {
+				return fmt.Errorf("expected warnings, got none")
+			}
+
+			if !strutil.StrListSubset(response.Warnings, warnings) {
+				return fmt.Errorf("expected response to contain the following warnings:\n%s\ngot:\n%s", warnings, response.Warnings)
+			}
+			return nil
+		},
+	}
+}
+
 func testAccStepGroup(t *testing.T, group string, policies string) logicaltest.TestStep {
 	t.Logf("[testAccStepGroup] - Registering group %s, policy %s", group, policies)
 	return logicaltest.TestStep{

builtin/credential/ldap/path_config.go

@@ -2,6 +2,7 @@ package ldap
 
 import (
 	"context"
+	"strings"
 
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/helper/consts"
@@ -10,6 +11,8 @@ import (
 	"github.com/hashicorp/vault/sdk/logical"
 )
 
+const userFilterWarning = "userfilter configured does not consider userattr and may result in colliding entity aliases on logins"
+
 func pathConfig(b *backend) *framework.Path {
 	p := &framework.Path{
 		Pattern: `config`,
@@ -110,9 +113,38 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, d *f
 	data := cfg.PasswordlessMap()
 	cfg.PopulateTokenData(data)
 
-	return &logical.Response{
+	resp := &logical.Response{
 		Data: data,
-	}, nil
+	}
+
+	if warnings := b.checkConfigUserFilter(cfg); len(warnings) > 0 {
+		resp.Warnings = warnings
+	}
+
+	return resp, nil
+}
+
+// checkConfigUserFilter performs a best-effort check the config's userfilter.
+// It will checked whether the templated or literal userattr value is present,
+// and if not return a warning.
+func (b *backend) checkConfigUserFilter(cfg *ldapConfigEntry) []string {
+	if cfg == nil || cfg.UserFilter == "" {
+		return nil
+	}
+
+	var warnings []string
+
+	switch {
+	case strings.Contains(cfg.UserFilter, "{{.UserAttr}}"):
+		// Case where the templated userattr value is provided
+	case strings.Contains(cfg.UserFilter, cfg.UserAttr):
+		// Case where the literal userattr value is provided
+	default:
+		b.Logger().Debug(userFilterWarning, "userfilter", cfg.UserFilter, "userattr", cfg.UserAttr)
+		warnings = append(warnings, userFilterWarning)
+	}
+
+	return warnings
 }
 
 func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
@@ -154,6 +186,12 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, d *
 		return nil, err
 	}
 
+	if warnings := b.checkConfigUserFilter(cfg); len(warnings) > 0 {
+		return &logical.Response{
+			Warnings: warnings,
+		}, nil
+	}
+
 	return nil, nil
 }
 

changelog/14095.txt

@@ -0,0 +1,4 @@
+```release-note:improvement
+auth/ldap: Add a response warning and server log whenever the config is accessed
+if `userfilter` doesn't consider `userattr`
+```

website/content/api-docs/auth/ldap.mdx

@@ -90,6 +90,8 @@ This endpoint configures the LDAP auth method.
 
 @include 'tokenfields.mdx'
 
+@include 'ldap-auth-userfilter-warning.mdx'
+
 ### Sample Request
 
 ```shell-session

website/content/docs/auth/ldap.mdx

@@ -118,7 +118,7 @@ There are two alternate methods of resolving the user object used to authenticat
 - `userattr` (string, optional) - Attribute on user attribute object matching the username passed when authenticating. Examples: `sAMAccountName`, `cn`, `uid`
 - `userfilter` (string, optional) - Go template used to construct a ldap user search filter. The template can access the following context variables: \[`UserAttr`, `Username`\]. The default userfilter is `({{.UserAttr}}={{.Username}})` or `(userPrincipalName={{.Username}}@UPNDomain)` if the `upndomain` parameter is set. The user search filter can be used to restrict what user can attempt to log in. For example, to limit login to users that are not contractors, you could write `(&(objectClass=user)({{.UserAttr}}={{.Username}})(!(employeeType=Contractor)))`.
 
-
+@include 'ldap-auth-userfilter-warning.mdx'
 
 #### Binding - Anonymous Search
 
@@ -129,6 +129,8 @@ There are two alternate methods of resolving the user object used to authenticat
 - `deny_null_bind` (bool, optional) - This option prevents users from bypassing authentication when providing an empty password. The default is `true`.
 - `anonymous_group_search` (bool, optional) - Use anonymous binds when performing LDAP group searches. Defaults to `false`.
 
+@include 'ldap-auth-userfilter-warning.mdx'
+
 #### Binding - User Principal Name (AD)
 
 - `upndomain` (string, optional) - userPrincipalDomain used to construct the UPN string for the authenticating user. The constructed UPN will appear as `[username]@UPNDomain`. Example: `example.com`, which will cause vault to bind as `username@example.com`.

website/content/partials/ldap-auth-userfilter-warning.mdx

@@ -0,0 +1,4 @@
+~> When specifying a `userfilter`, either the templated value `{{.UserAttr}}` or
+the literal value that matches `userattr` should be present in the filter to
+ensure that the search returns a unique result that takes `userattr` into
+consideration for entity alias mapping purposes and avoid possible collisions on login.