Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of Fix handling of username_as_alias during LDAP authentication into release/1.10.x #15557

Merged
6 changes: 5 additions & 1 deletion builtin/credential/ldap/backend.go
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions builtin/credential/ldap/backend_test.go
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
},
}
}
Expand Down
8 changes: 2 additions & 6 deletions builtin/credential/ldap/path_login.go
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 8 additions & 0 deletions 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.
```
2 changes: 1 addition & 1 deletion sdk/helper/ldaputil/client.go
Expand Up @@ -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 {
Expand Down