Skip to content

Support LDAP authentication with unauthorized reads#740

Closed
JonasScharpf wants to merge 6 commits intobuchgr:masterfrom
JonasScharpf:feature/support-ldap
Closed

Support LDAP authentication with unauthorized reads#740
JonasScharpf wants to merge 6 commits intobuchgr:masterfrom
JonasScharpf:feature/support-ldap

Conversation

@JonasScharpf
Copy link
Copy Markdown
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

@JonasScharpf JonasScharpf force-pushed the feature/support-ldap branch from faecd41 to 2452267 Compare April 2, 2024 15:22
Copy link
Copy Markdown
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.

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread config/config.go
Comment thread config/config.go Outdated
Comment thread config/config.go Outdated
Comment thread config/config.go Outdated
Comment thread main.go Outdated
Comment thread ldap/ldap.go Outdated
Comment thread main.go Outdated
* use LDAP fake server package
* expose config from YAML function
* update go repository and deps list
@JonasScharpf JonasScharpf force-pushed the feature/support-ldap branch from 4ef6685 to e2a1d74 Compare April 18, 2024 09:09
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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