diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index 37ba20ae6c09e..bbd9f1bfc157d 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -56,7 +56,7 @@ type backend struct { *framework.Backend } -func (b *backend) Login(ctx context.Context, req *logical.Request, username string, password string) (string, []string, *logical.Response, []string, error) { +func (b *backend) Login(ctx context.Context, req *logical.Request, username string, password string, usernameAsAlias bool) (string, []string, *logical.Response, []string, error) { cfg, err := b.Config(ctx, req) if err != nil { return "", nil, nil, nil, err @@ -195,6 +195,10 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri // Policies from each group may overlap policies = strutil.RemoveDuplicates(policies, true) + if usernameAsAlias { + return username, policies, ldapResponse, allGroups, nil + } + entityAliasAttribute, err := ldapClient.GetUserAliasAttributeValue(cfg.ConfigEntry, c, username) if err != nil { return "", nil, logical.ErrorResponse(err.Error()), nil, nil diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index c58fa2f2d10a7..7a9f0f0eed5e3 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -618,6 +618,30 @@ func TestBackend_basic_authbind_userfilter(t *testing.T) { testAccStepLoginFailure(t, "hermes conrad", "hermes"), }, }) + + // If UserAttr returns multiple attributes that can be used as alias then + // we return an error... + cfg.UserAttr = "employeeType" + cfg.UserFilter = "(cn={{.Username}})" + cfg.UsernameAsAlias = false + logicaltest.Test(t, logicaltest.TestCase{ + CredentialBackend: b, + Steps: []logicaltest.TestStep{ + testAccStepConfigUrl(t, cfg), + testAccStepLoginFailure(t, "hermes conrad", "hermes"), + }, + }) + + // ...unless username_as_alias has been set in which case we don't care + // about the alias returned by the LDAP server and always use the username + cfg.UsernameAsAlias = true + logicaltest.Test(t, logicaltest.TestCase{ + CredentialBackend: b, + Steps: []logicaltest.TestStep{ + testAccStepConfigUrl(t, cfg), + testAccStepLoginNoAttachedPolicies(t, "hermes conrad", "hermes"), + }, + }) } func TestBackend_basic_authbind_metadata_name(t *testing.T) { @@ -805,6 +829,7 @@ func testAccStepConfigUrl(t *testing.T, cfg *ldaputil.ConfigEntry) logicaltest.T "case_sensitive_names": true, "token_policies": "abc,xyz", "request_timeout": cfg.RequestTimeout, + "username_as_alias": cfg.UsernameAsAlias, }, } } diff --git a/builtin/credential/ldap/path_login.go b/builtin/credential/ldap/path_login.go index 49c0cfe9d8fb1..67303911e5a17 100644 --- a/builtin/credential/ldap/path_login.go +++ b/builtin/credential/ldap/path_login.go @@ -73,7 +73,7 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, d *framew username := d.Get("username").(string) password := d.Get("password").(string) - effectiveUsername, policies, resp, groupNames, err := b.Login(ctx, req, username, password) + effectiveUsername, policies, resp, groupNames, err := b.Login(ctx, req, username, password, cfg.UsernameAsAlias) // Handle an internal error if err != nil { return nil, err @@ -103,10 +103,6 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, d *framew }, } - if cfg.UsernameAsAlias { - auth.Alias.Name = username - } - cfg.PopulateTokenAuth(auth) // Add in configured policies from mappings @@ -139,7 +135,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f username := req.Auth.Metadata["username"] password := req.Auth.InternalData["password"].(string) - _, loginPolicies, resp, groupNames, err := b.Login(ctx, req, username, password) + _, loginPolicies, resp, groupNames, err := b.Login(ctx, req, username, password, cfg.UsernameAsAlias) if err != nil || (resp != nil && resp.IsError()) { return resp, err } diff --git a/changelog/15525.txt b/changelog/15525.txt new file mode 100644 index 0000000000000..3e44c205b18ff --- /dev/null +++ b/changelog/15525.txt @@ -0,0 +1,8 @@ +```release-note:bug +auth/ldap: The logic for setting the entity alias when `username_as_alias` is set +has been fixed. The previous behavior would make a request to the LDAP server to +get `user_attr` before discarding it and using the username instead. This would +make it impossible for a user to connect if this attribute was missing or had +multiple values, even though it would not be used anyway. This has been fixed +and the username is now used without making superfluous LDAP searches. +``` diff --git a/sdk/helper/ldaputil/client.go b/sdk/helper/ldaputil/client.go index 329e69ecc8e54..f3946c8269e2c 100644 --- a/sdk/helper/ldaputil/client.go +++ b/sdk/helper/ldaputil/client.go @@ -244,7 +244,7 @@ func (c *Client) GetUserAliasAttributeValue(cfg *ConfigEntry, conn Connection, u } if len(result.Entries[0].Attributes) != 1 { - return aliasAttributeValue, errwrap.Wrapf("LDAP attribute missing for entity alias mapping{{err}}", err) + return aliasAttributeValue, fmt.Errorf("LDAP attribute missing for entity alias mapping") } if len(result.Entries[0].Attributes[0].Values) != 1 {