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

Update LDAP user search test #620

Merged
merged 5 commits into from
Oct 3, 2024
Merged

Conversation

bhavenst
Copy link
Contributor

@bhavenst bhavenst commented Sep 26, 2024

  • Blank UI field for user search results in the value []
  • Current test to ignore is "if field is None:", which fails since [] is not None
  • With "if field:", [] is False and properly ignored

LDAP auth should require one of User Search or User DN Template, but not both. Currently LDAP auth is blocked if User Search is empty ([]), this change fixes this issue and allows auth when only User DN Template is specified.

This is for: https://issues.redhat.com/browse/AAP-28020

That Jira is now closed, but this is good to fix anyway.

 * Blank UI field for user search results in the value []
 * Current test to ignore is "if field is None:", which fails since []
   is not None
 * With "if field:", [] is False and properly ignored
@bhavenst bhavenst requested a review from tznamena September 26, 2024 17:37
@bhavenst bhavenst self-assigned this Sep 26, 2024
@bhavenst bhavenst added the Ready for review This PR is ready for review either initially or comments have been address label Sep 26, 2024
@tznamena
Copy link
Contributor

Shouldn't we add a test for this as well?

@bhavenst
Copy link
Contributor Author

Yeah, was in progress.. Added a case for it now.

@bhavenst
Copy link
Contributor Author

Oops, test didn't hit new code.. Standby..

@bhavenst
Copy link
Contributor Author

OK, unit test added to cover this strange scenario.

Copy link
Contributor

@art-tapin art-tapin left a comment

Choose a reason for hiding this comment

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

🥇

ansible_base/authentication/authenticator_plugins/ldap.py Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Oct 2, 2024

@bhavenst bhavenst merged commit 4a38ba0 into ansible:devel Oct 3, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for review This PR is ready for review either initially or comments have been address
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants