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

Support LDAP authentication with unauthorized reads #740

Closed
wants to merge 6 commits into from

Conversation

JonasScharpf
Copy link
Contributor

This PR is based on #101 and resolves #100

It provides unauthorized reads when using LDAP, config via command line args and minimal config file testing for the new LDAP section

This is my first GO thing ever, so it might not be perfect

Copy link
Collaborator

@mostynb mostynb left a comment

Choose a reason for hiding this comment

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

Not bad, for first-time go code :) I will look some more tomorrow.

I wonder if there's a small ldap server we could spin up during an end-to-end test? https://github.com/bradleypeabody/godap might work.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
config/config.go Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
ldap/ldap.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
* use LDAP fake server package
* expose config from YAML function
* update go repository and deps list
Copy link
Contributor Author

@JonasScharpf JonasScharpf left a comment

Choose a reason for hiding this comment

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

so I've fixed your remarks and added a end-to-end fake LDAP server test. But the remaining golangci linter errors are beyond my knowledge to fix them by myself.
I have to admit that I simply copied the httpServer thing from an example ...

@mostynb
Copy link
Collaborator

mostynb commented Apr 24, 2024

so I've fixed your remarks and added a end-to-end fake LDAP server test. But the remaining golangci linter errors are beyond my knowledge to fix them by myself. I have to admit that I simply copied the httpServer thing from an example ...

Sorry it took me longer than expected to get back to this. I should have some time to read through this while I'm travelling later this week.

@JonasScharpf
Copy link
Contributor Author

Hey @mostynb how are you doing? Might it be possible to find some time for this PR during your next trip? 😉

@JonasScharpf
Copy link
Contributor Author

@mostynb I'm sorry to ask again for your time + help with this PR, it's been now in review since almost 2 months

@mostynb
Copy link
Collaborator

mostynb commented Aug 11, 2024

Sorry this took me so long. I landed a modified version of this in #768. Could you please try it out?

Some of the bigger changes:

  • I replaced ldap_tests.go with a shell script that uses https://github.com/glauth/glauth (.bazelci/ldap-tests.sh - it unfortunately only runs on linux at the moment)
  • I had to add the BaseDN when calling Bind
  • I changed the ldap.groups (slice) setting to a ldap.groups_query string

@mostynb mostynb closed this Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support LDAP authentication
2 participants