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

updated the ldap bind functions to provide for anonymous binds #177

Merged
merged 4 commits into from
Jan 22, 2025

Conversation

babagreensheep
Copy link
Contributor

@ViViDboarder
Copy link
Owner

Thanks for the PR! Do you think you could build an integration test case? I could probably help out with it later too.

@babagreensheep
Copy link
Contributor Author

Sorry, I'm not quite sure how to do that? This is my first time contributing to open source and I'd love any guidance!

@ViViDboarder
Copy link
Owner

Just made some changes to make it easier. If you're more familiar than I am with ldap, you can add an ldif file to the itest/ldif directory. That ldiff will need to give anon bind read access. I tried this but couldn't get slapd to start, but I'm not very familiar with ldap.

Once that's working, you'll need to create a new configuration to attempt anon-binding. The easiest way would be to copy itest/docker-compose.itest-env.yml and call it something like docker-compose.itest-anon.yml and remove the bind username and passwords. Then copy the itest-env target in the Makefile calling it something like itest-anon and change the compose file to the new one.

From there you can do make itest-anon and you should be able to debug from there.

@ViViDboarder ViViDboarder self-assigned this Nov 1, 2024
@ViViDboarder ViViDboarder added the enhancement New feature or request label Nov 1, 2024
Copy link
Owner

@ViViDboarder ViViDboarder left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution and congrats on your first open source experience! I've provided some feedback that should fix a bug as well as make it easier to maintain. If you have any questions, I'm happy to help out.

@babagreensheep
Copy link
Contributor Author

Okay got it let me work on the test (and fix non-idiomatic code: yes I was lazy earlier).

Re anon bind: in all my tests (not just with this library but other services I'm working on) I didn't necessarily have to bind to empty strings and in fact on the Rust library I ran into some issues binding with empty strings. I will try to see if I can bottom this out.

Thanks for helping a first timer out!!

@babagreensheep babagreensheep deleted the anon branch November 21, 2024 07:21
@babagreensheep babagreensheep restored the anon branch November 21, 2024 07:21
@babagreensheep
Copy link
Contributor Author

babagreensheep commented Nov 21, 2024

Hi! I've added a new test (itest-anon) in the makefile and the tests appears to run successfully. I've also updated fixed the non-idiomatic pattern and added errors for invalid authentication (username but no pw and vice versa). I'm not sure if i have correctly pushed it though?

Edit :I'm not sure how to push the newly updated branch up... any guidance would be very helpful!

@ViViDboarder
Copy link
Owner

Thanks for your contribution! I ran into itest issues relating to the ldap image that I was using, so I switched images and then had to rebase your change on top as well. The new image allows anon by default, simplifying things a lot too.

@ViViDboarder ViViDboarder merged commit f983cb3 into ViViDboarder:main Jan 22, 2025
2 checks passed
@ZephOne ZephOne mentioned this pull request Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants