Skip to content

update alias lookahead to respect case#31352

Merged
benashz merged 9 commits into
mainfrom
alias-lookahead-case
Jul 23, 2025
Merged

update alias lookahead to respect case#31352
benashz merged 9 commits into
mainfrom
alias-lookahead-case

Conversation

@mickael-hc
Copy link
Copy Markdown
Contributor

Description

update alias lookahead to respect case:

  • userpass is not case sensitive
  • ldap is case sensitive when it is configured that way

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

* userpass is not case sensitive
* ldap is case sensitive when it is configured that way
@mickael-hc mickael-hc requested review from a team as code owners July 22, 2025 14:00
@github-actions github-actions Bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 22, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 22, 2025

CI Results:
All Go tests succeeded! ✅

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 22, 2025

Build Results:
All builds succeeded! ✅

bosouza
bosouza previously approved these changes Jul 22, 2025
Copy link
Copy Markdown
Contributor

@bosouza bosouza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Comment thread builtin/credential/ldap/path_login.go
Comment thread builtin/credential/ldap/path_login.go Outdated
if err != nil {
return nil, err
}
if !*cfg.CaseSensitiveNames {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like another potential nil pointer dereference. You will want to guard against this runtime error with a nil check.

Comment thread builtin/credential/ldap/path_login.go

func (b *backend) pathLoginAliasLookahead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
username := d.Get("username").(string)
username := strings.ToLower(d.Get("username").(string))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some tests for this change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in 3b2636f

check for nil and add some tests
@mickael-hc mickael-hc force-pushed the alias-lookahead-case branch from bf2310e to c1ab541 Compare July 23, 2025 03:04
@mickael-hc mickael-hc requested a review from benashz July 23, 2025 03:07
benashz
benashz previously approved these changes Jul 23, 2025
tomcf-hcp
tomcf-hcp previously approved these changes Jul 23, 2025
@benashz benashz enabled auto-merge (squash) July 23, 2025 16:03
@benashz benashz disabled auto-merge July 23, 2025 16:08
@benashz benashz enabled auto-merge (squash) July 23, 2025 16:09
@benashz benashz merged commit 881febb into main Jul 23, 2025
91 checks passed
@benashz benashz deleted the alias-lookahead-case branch July 23, 2025 16:28
Erfankam pushed a commit to Erfankam/vault that referenced this pull request Sep 1, 2025
* userpass is not case sensitive
* ldap is case sensitive when it is configured that way

---------

Co-authored-by: Ben Ash <bash@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants