Skip to content

Commit 299306f

Browse files
mickael-hcbenashz
andauthored
auth/ldap: ensure consistent entity aliasing when set from the username (#31427) (#31430)
[ent: a552ac1e80e3d334673c59a5bb825082cd56b1bf] Co-authored-by: Ben Ash <[email protected]>
1 parent 69fcd7e commit 299306f

File tree

3 files changed

+79
-29
lines changed

3 files changed

+79
-29
lines changed

builtin/credential/ldap/backend.go

Lines changed: 59 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010
"sync"
1111

12+
goldap "github.com/go-ldap/ldap/v3"
1213
"github.com/hashicorp/cap/ldap"
1314
"github.com/hashicorp/go-secure-stdlib/strutil"
1415
"github.com/hashicorp/vault/sdk/framework"
@@ -69,6 +70,14 @@ type backend struct {
6970
mu sync.RWMutex
7071
}
7172

73+
func (b *backend) maybeLogDebug(msg string, args ...interface{}) {
74+
if b.Logger().IsDebug() {
75+
b.Logger().Debug(msg, args...)
76+
}
77+
}
78+
79+
// Login authenticates a user against the LDAP server using the provided username and password.
80+
// It returns the effective username, policies associated with the user, a response containing
7281
func (b *backend) Login(ctx context.Context, req *logical.Request, username string, password string, usernameAsAlias bool) (string, []string, *logical.Response, []string, error) {
7382
cfg, err := b.Config(ctx, req)
7483
if err != nil {
@@ -84,9 +93,7 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
8493

8594
ldapClient, err := ldap.NewClient(ctx, ldaputil.ConvertConfig(cfg.ConfigEntry))
8695
if err != nil {
87-
if b.Logger().IsDebug() {
88-
b.Logger().Debug("error creating client", "error", err)
89-
}
96+
b.maybeLogDebug("error creating client", "error", err)
9097
return "", nil, logical.ErrorResponse(err.Error()), nil, nil
9198
}
9299

@@ -97,17 +104,16 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
97104
if err != nil {
98105
if strings.Contains(err.Error(), "discovery of user bind DN failed") ||
99106
strings.Contains(err.Error(), "unable to bind user") {
100-
if b.Logger().IsDebug() {
101-
b.Logger().Debug("error getting user bind DN", "error", err)
102-
}
107+
b.maybeLogDebug("error getting user bind DN", username, "error", err)
103108
return "", nil, logical.ErrorResponse(errUserBindFailed), nil, logical.ErrInvalidCredentials
104109
}
105110

106111
return "", nil, logical.ErrorResponse(err.Error()), nil, nil
107112
}
108113

109-
if b.Logger().IsDebug() {
110-
b.Logger().Debug("user binddn fetched", "username", username, "binddn", c.UserDN)
114+
b.maybeLogDebug("user binddn fetched", "username", username, "binddn", c.UserDN)
115+
if c.UserDN == "" {
116+
return "", nil, logical.ErrorResponse("user bind DN is empty after authentication"), nil, nil
111117
}
112118

113119
ldapGroups := c.Groups
@@ -119,36 +125,59 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
119125
"no LDAP groups found in groupDN %q; only policies from locally-defined groups available",
120126
cfg.GroupDN)
121127

122-
if b.Logger().IsDebug() {
123-
b.Logger().Debug(errString)
124-
}
128+
b.maybeLogDebug(errString)
125129
}
126130

127131
for _, warning := range c.Warnings {
128-
if b.Logger().IsDebug() {
129-
b.Logger().Debug(string(warning))
132+
b.maybeLogDebug(string(warning))
133+
}
134+
135+
canonicalUsername := username
136+
if usernameAsAlias {
137+
// expect to get the username from UserDN in the case where we are setting the
138+
// entity alias to be the username.
139+
parsed, err := goldap.ParseDN(c.UserDN)
140+
if err != nil {
141+
b.Logger().Error("Invalid DN after authentication", "user_dn", c.UserDN, "error", err)
142+
return "", nil, logical.ErrorResponse("invalid DN after authentication"), nil, nil
143+
}
144+
145+
b.maybeLogDebug("User DN parsed", "parsed", parsed, "username", username)
146+
var found bool
147+
for _, rdn := range parsed.RDNs {
148+
if found {
149+
b.maybeLogDebug("Found canonical username", "canonicalUsername", canonicalUsername, "cfg.userAttr", cfg.UserAttr)
150+
break
151+
}
152+
for _, attr := range rdn.Attributes {
153+
b.maybeLogDebug("Handling RDN", "attr.Type", attr.Type, "attr.Value", attr.Value, "cfg.UserAttr", cfg.UserAttr)
154+
if strings.EqualFold(attr.Type, cfg.UserAttr) {
155+
b.maybeLogDebug("Found user attribute in RDN", "attr.Type", attr.Type, "attr.Value", attr.Value)
156+
canonicalUsername = attr.Value
157+
found = true
158+
break
159+
}
160+
}
130161
}
131162
}
132163

133164
var allGroups []string
134-
canonicalUsername := username
135-
cs := *cfg.CaseSensitiveNames
165+
166+
cs := cfg.CaseSensitiveNames != nil && *cfg.CaseSensitiveNames
136167
if !cs {
137-
canonicalUsername = strings.ToLower(username)
168+
canonicalUsername = strings.ToLower(canonicalUsername)
138169
}
170+
139171
// Import the custom added groups from ldap backend
140172
user, err := b.User(ctx, req.Storage, canonicalUsername)
141173
if err == nil && user != nil && user.Groups != nil {
142-
if b.Logger().IsDebug() {
143-
b.Logger().Debug("adding local groups", "num_local_groups", len(user.Groups), "local_groups", user.Groups)
144-
}
145174
allGroups = append(allGroups, user.Groups...)
146175
}
147176
// Merge local and LDAP groups
148177
allGroups = append(allGroups, ldapGroups...)
149178

150179
canonicalGroups := allGroups
151-
// If not case sensitive, lowercase all
180+
// If not case-sensitive, lowercase all
152181
if !cs {
153182
canonicalGroups = make([]string, len(allGroups))
154183
for i, v := range allGroups {
@@ -171,19 +200,23 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
171200
policies = strutil.RemoveDuplicates(policies, true)
172201

173202
if usernameAsAlias {
174-
return username, policies, ldapResponse, allGroups, nil
203+
b.maybeLogDebug("UsernameAlias is set", "canonicalUsername", canonicalUsername, "policies", policies, "groups", allGroups)
204+
return canonicalUsername, policies, ldapResponse, allGroups, nil
175205
}
176206

177207
userAttrValues := c.UserAttributes[cfg.UserAttr]
178208
if len(userAttrValues) == 0 {
179-
if b.Logger().IsDebug() {
180-
b.Logger().Debug("missing entity alias attribute value")
181-
}
209+
b.Logger().Error("missing entity alias attribute value")
182210
return "", nil, logical.ErrorResponse("missing entity alias attribute value"), nil, nil
183211
}
184-
entityAliasAttribute := userAttrValues[0]
185212

186-
return entityAliasAttribute, policies, ldapResponse, allGroups, nil
213+
effectiveUsername := userAttrValues[0]
214+
if effectiveUsername == "" {
215+
b.Logger().Error("empty entity alias attribute value")
216+
return "", nil, logical.ErrorResponse("empty entity alias attribute value"), nil, nil
217+
}
218+
219+
return effectiveUsername, policies, ldapResponse, allGroups, nil
187220
}
188221

189222
const backendHelp = `

builtin/credential/ldap/path_login.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,16 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, d *framew
9494
password := d.Get("password").(string)
9595

9696
effectiveUsername, policies, resp, groupNames, err := b.Login(ctx, req, username, password, cfg.UsernameAsAlias)
97-
if err != nil || (resp != nil && resp.IsError()) {
98-
return resp, err
97+
if err != nil {
98+
return nil, err
99+
}
100+
101+
if resp != nil {
102+
if resp.IsError() {
103+
return nil, resp.Error()
104+
}
105+
} else {
106+
return nil, fmt.Errorf("login response is nil, this should not happen")
99107
}
100108

101109
auth := &logical.Auth{
@@ -109,7 +117,16 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, d *framew
109117
Alias: &logical.Alias{
110118
Name: effectiveUsername,
111119
Metadata: map[string]string{
120+
// Should be the same as raw username, but we store it here for posterity.
112121
"name": username,
122+
// We store the original username in the metadata so that we can
123+
// reference it later if needed, such as in the alias lookahead.
124+
// This is useful for cases where the username is transformed or
125+
// normalized (e.g., lowercased) for authentication purposes.
126+
"rawUsername": username,
127+
// The effective username is the one that will be used for policies and aliases.
128+
// This may differ from the raw username if transformations are applied.
129+
"effectiveUsername": effectiveUsername,
113130
},
114131
},
115132
}

sdk/helper/ldaputil/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ func (c *Client) GetLdapGroups(cfg *ConfigEntry, conn Connection, userDN string,
622622
}
623623

624624
// EscapeLDAPValue is exported because a plugin uses it outside this package.
625-
// EscapeLDAPValue will properly escape the input string as an ldap value
625+
// EscapeLDAPValue will properly escape the input string as a ldap value
626626
// rfc4514 states the following must be escaped:
627627
// - leading space or hash
628628
// - trailing space

0 commit comments

Comments
 (0)