From d757183cf342c471ce3528c84bef37b55a2b0bc7 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Fri, 20 May 2022 18:48:16 -0400 Subject: [PATCH] Backport of Fix handling of username_as_alias during LDAP authentication into release/1.10.x (#15557) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * backport of commit 1bd9e76e13379539bc396899729453f6f2c9aa1f * backport of commit 92be3035b7c750ea3d16447297f60fc413794ac8 * backport of commit ac7c85c3de59082a6d3598211629b62010d604f2 * backport of commit ae7e2e91ae19660e879f592c07ff42c1be50c499 * backport of commit 04d8cb9acdae88c484f303aad9751177f93a0c97 * backport of commit 3a713c1004d6ec1035c6be35b8ad4eb48dbc3c44 * backport of commit a741e0de15c709795bbc12c5df7ca4fc9e48e93c Co-authored-by: Rémi Lapeyre Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> --- builtin/credential/ldap/backend.go | 6 +++++- builtin/credential/ldap/backend_test.go | 25 +++++++++++++++++++++++++ builtin/credential/ldap/path_login.go | 8 ++------ changelog/15525.txt | 8 ++++++++ sdk/helper/ldaputil/client.go | 2 +- 5 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 changelog/15525.txt diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index 554f103ebc7c5..c02fe93f871d7 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -60,7 +60,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 @@ -199,6 +199,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 {