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.9.x #15556

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 {
Expand Down
41 changes: 31 additions & 10 deletions builtin/credential/ldap/backend_test.go
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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}}))"

Expand All @@ -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")
Expand All @@ -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{
Expand All @@ -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()
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
},
}
}
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