From c2d280a89a33283271f8fd7a74b7d21f1e88e8f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Thu, 19 May 2022 22:07:23 +0000 Subject: [PATCH 1/7] backport of commit 1bd9e76e13379539bc396899729453f6f2c9aa1f --- builtin/credential/ldap/backend.go | 17 ++++++++++------- builtin/credential/ldap/path_login.go | 8 ++------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index 554f103ebc7c5..bf440e72c74c0 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,12 +199,15 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri // Policies from each group may overlap policies = strutil.RemoveDuplicates(policies, true) - entityAliasAttribute, err := ldapClient.GetUserAliasAttributeValue(cfg.ConfigEntry, c, username) - if err != nil { - return "", nil, logical.ErrorResponse(err.Error()), nil, nil - } - if entityAliasAttribute == "" { - return "", nil, logical.ErrorResponse("missing entity alias attribute value"), nil, nil + entityAliasAttribute := username + if !usernameAsAlias { + entityAliasAttribute, err = ldapClient.GetUserAliasAttributeValue(cfg.ConfigEntry, c, username) + if err != nil { + return "", nil, logical.ErrorResponse(err.Error()), nil, nil + } + if entityAliasAttribute == "" { + return "", nil, logical.ErrorResponse("missing entity alias attribute value"), nil, nil + } } return entityAliasAttribute, policies, ldapResponse, allGroups, nil 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 } From c64edf44d55d165c94317082a0cc2e0b62bc014c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Thu, 19 May 2022 22:29:59 +0000 Subject: [PATCH 2/7] backport of commit 92be3035b7c750ea3d16447297f60fc413794ac8 --- changelog/15525.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/15525.txt diff --git a/changelog/15525.txt b/changelog/15525.txt new file mode 100644 index 0000000000000..a4b23820efb55 --- /dev/null +++ b/changelog/15525.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/ldap: A bug during login has been fixed when `username_as_alias` is set. +``` From 1aade36d0a1c1ea320945c3543e5f2da86a95af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Thu, 19 May 2022 22:45:25 +0000 Subject: [PATCH 3/7] backport of commit ac7c85c3de59082a6d3598211629b62010d604f2 --- builtin/credential/ldap/backend_test.go | 26 +++++++++++++++++++++++++ sdk/helper/ldaputil/client.go | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index c58fa2f2d10a7..77ab29648d723 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -618,6 +618,31 @@ 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 +830,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/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 { From 15db7cad02d873c4689a257791538f514632f65c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Thu, 19 May 2022 23:02:54 +0000 Subject: [PATCH 4/7] backport of commit ae7e2e91ae19660e879f592c07ff42c1be50c499 --- builtin/credential/ldap/backend_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index 77ab29648d723..7a9f0f0eed5e3 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -642,7 +642,6 @@ func TestBackend_basic_authbind_userfilter(t *testing.T) { testAccStepLoginNoAttachedPolicies(t, "hermes conrad", "hermes"), }, }) - } func TestBackend_basic_authbind_metadata_name(t *testing.T) { From 1827a17ba9ca1001f99e34f274208ca674be06aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Fri, 20 May 2022 06:40:41 +0000 Subject: [PATCH 5/7] backport of commit 04d8cb9acdae88c484f303aad9751177f93a0c97 --- builtin/credential/ldap/backend.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index bf440e72c74c0..d45e9540f443c 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -199,15 +199,16 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri // Policies from each group may overlap policies = strutil.RemoveDuplicates(policies, true) - entityAliasAttribute := username - if !usernameAsAlias { - entityAliasAttribute, err = ldapClient.GetUserAliasAttributeValue(cfg.ConfigEntry, c, username) - if err != nil { - return "", nil, logical.ErrorResponse(err.Error()), nil, nil - } - if entityAliasAttribute == "" { - return "", nil, logical.ErrorResponse("missing entity alias attribute value"), nil, nil - } + 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 + } + if entityAliasAttribute == "" { + return "", nil, logical.ErrorResponse("missing entity alias attribute value"), nil, nil } return entityAliasAttribute, policies, ldapResponse, allGroups, nil From 5505b5988caa441fa6c847bd48ed22c5d39f662b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Fri, 20 May 2022 15:27:42 +0000 Subject: [PATCH 6/7] backport of commit 3a713c1004d6ec1035c6be35b8ad4eb48dbc3c44 --- builtin/credential/ldap/backend.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index d45e9540f443c..c02fe93f871d7 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -202,8 +202,8 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri if usernameAsAlias { return username, policies, ldapResponse, allGroups, nil } - - entityAliasAttribute, err = ldapClient.GetUserAliasAttributeValue(cfg.ConfigEntry, c, username) + + entityAliasAttribute, err := ldapClient.GetUserAliasAttributeValue(cfg.ConfigEntry, c, username) if err != nil { return "", nil, logical.ErrorResponse(err.Error()), nil, nil } From 5b65f22415e9bbeab4883908a9d44107b56c4b06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Fri, 20 May 2022 20:50:14 +0000 Subject: [PATCH 7/7] backport of commit a741e0de15c709795bbc12c5df7ca4fc9e48e93c --- changelog/15525.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/changelog/15525.txt b/changelog/15525.txt index a4b23820efb55..3e44c205b18ff 100644 --- a/changelog/15525.txt +++ b/changelog/15525.txt @@ -1,3 +1,8 @@ ```release-note:bug -auth/ldap: A bug during login has been fixed when `username_as_alias` is set. +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. ```