diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index 15e55fe15e754..d94df9db14b1b 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 { diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index 59c9366e9c32d..1c5fd91f61347 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -501,12 +501,11 @@ func TestBackend_basic_authbind(t *testing.T) { } func TestBackend_basic_authbind_userfilter(t *testing.T) { - b := factory(t) cleanup, cfg := ldap.PrepareTestContainer(t, "latest") defer cleanup() - //Add a liberal user filter, allowing to log in with either cn or email + // Add a liberal user filter, allowing to log in with either cn or email cfg.UserFilter = "(|({{.UserAttr}}={{.Username}})(mail={{.Username}}))" logicaltest.Test(t, logicaltest.TestCase{ @@ -524,7 +523,7 @@ func TestBackend_basic_authbind_userfilter(t *testing.T) { }, }) - //A filter giving the same DN makes the entity_id the same + // A filter giving the same DN makes the entity_id the same entity_id := "" logicaltest.Test(t, logicaltest.TestCase{ @@ -542,7 +541,7 @@ func TestBackend_basic_authbind_userfilter(t *testing.T) { }, }) - //Missing entity alias attribute means access denied + // Missing entity alias attribute means access denied cfg.UserAttr = "inexistent" cfg.UserFilter = "(|({{.UserAttr}}={{.Username}})(mail={{.Username}}))" @@ -556,7 +555,7 @@ func TestBackend_basic_authbind_userfilter(t *testing.T) { }) cfg.UserAttr = "cn" - //UPNDomain has precedence over userfilter, for backward compatibility + // UPNDomain has precedence over userfilter, for backward compatibility cfg.UPNDomain = "planetexpress.com" addUPNAttributeToLDAPSchemaAndUser(t, cfg, "cn=Hubert J. Farnsworth,ou=people,dc=planetexpress,dc=com", "professor@planetexpress.com") @@ -571,7 +570,7 @@ func TestBackend_basic_authbind_userfilter(t *testing.T) { cfg.UPNDomain = "" - //Add a strict user filter, rejecting login of bureaucrats + // Add a strict user filter, rejecting login of bureaucrats cfg.UserFilter = "(&({{.UserAttr}}={{.Username}})(!(employeeType=Bureaucrat)))" logicaltest.Test(t, logicaltest.TestCase{ @@ -583,22 +582,44 @@ func TestBackend_basic_authbind_userfilter(t *testing.T) { }, }) - //Login fails when multiple user match search filter (using an incorrect filter on purporse) + // Login fails when multiple user match search filter (using an incorrect filter on purporse) cfg.UserFilter = "(objectClass=*)" logicaltest.Test(t, logicaltest.TestCase{ CredentialBackend: b, Steps: []logicaltest.TestStep{ - //testAccStepConfigUrl(t, cfg), + // testAccStepConfigUrl(t, cfg), testAccStepConfigUrlWithAuthBind(t, cfg), // Authenticate with cn attribute 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) { - b := factory(t) cleanup, cfg := ldap.PrepareTestContainer(t, "latest") defer cleanup() @@ -661,7 +682,6 @@ func addUPNAttributeToLDAPSchemaAndUser(t *testing.T, cfg *ldaputil.ConfigEntry, if err := conn.Modify(modifyUserReq); err != nil { t.Fatal(err) } - } func TestBackend_basic_discover(t *testing.T) { @@ -784,6 +804,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 2c3a17fb98cee..8e07d2282b6cb 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 {