Conversation
|
👋 CL-Andrew, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
|
✅ No conflicts with other open PRs targeting |
|
I see you updated files related to
|
There was a problem hiding this comment.
Pull request overview
Risk Rating: MEDIUM — touches authentication/bind behavior; small diff but can impact logins for certain valid usernames.
Updates LDAP DN escaping used when constructing the bind DN for user authentication in the LDAP auth provider.
Changes:
- Switch from
ldap.EscapeFilter(...)toldap.EscapeDN(...)when building the user bind DN inCreateSession. - Switch from
ldap.EscapeFilter(...)toldap.EscapeDN(...)when building the user bind DN inTestPassword.
| // Attempt to LDAP Bind with user provided credentials | ||
| escapedEmail := ldap.EscapeFilter(strings.ToLower(sr.Email)) | ||
| escapedEmail := ldap.EscapeDN(strings.ToLower(sr.Email)) | ||
| searchBaseDN := fmt.Sprintf("%s=%s,%s,%s", l.config.BaseUserAttr(), escapedEmail, l.config.UsersDN(), l.config.BaseDN()) | ||
| if err = conn.Bind(searchBaseDN, sr.Password); err != nil { | ||
| l.lggr.Infof("Error binding user authentication request in LDAP Bind: %v", err) |
There was a problem hiding this comment.
escapedEmail is now produced via ldap.EscapeDN(...) for use in the bind DN. Ensure this DN-escaped value is not reused later as the logical user email (e.g., passed into FindUser / used for local DB lookups), because DN escaping can change valid emails (notably + in plus-addressing) and will cause user/group lookup to fail. Keep separate variables: normalized email for identity/DB and DN-escaped value solely for building the bind DN.
|
No description provided.