-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fixed dynamic group ldap access #23450
Conversation
getUserGroups: Using $userDN instead of $uid to query LDAP Converting groupDN to group name using API instead of substring Removing cache processing at the end of the method
added back the cache processing and fixed
spaces -> tab conversion
By analyzing the blame information on this pull request, we identified @alexweirig, @leo-b and @GreenArchon to be potential reviewers |
@SergioBertolinSG @davitol @blizzz Could you check this out? |
We don't have a ldap server with dynamic groups. Last time I tried to set it up with @blizzz many problems arose, There is an ldap server with dynamic groups in an already prepared docker which we can use? Also mentioning @davicente here. |
I have no idea, but maybe @alexweirig has an idea how to test this (or how to fire up a useful LDAP server) :) |
@SergioBertolinSG it took me 5 minutes with a fresh docker instance… |
Please share your knowledge and drop it in the wiki :) |
Based on a docker-image from private repo. Thus I provided a pastebin to @SergioBertolinSG on IRC in middle of April, but know it's run out of course. Need to do it again. |
THen maybe build it in open source 🙈 |
Not my decision :p |
With that docker containter, the LDAP server must be startet first (1. command), then the schema needs to be added (2. command) and finally a dynamic group needs to be created, in this example it will have all users underneath a certain base as members (3. command) |
When I use second command inside the container I get:
@blizzz how are you running this commands? (this is using a fresh new container) |
All I did outside of docker was
Everything else inside docker describe in #23450 (comment) Nothing else. That's all. |
Works fine 👍 |
My vote was on the original PR: #23344 (comment) |
And Contributor signed the CLA meanwhile. |
Needs backport to 9.0, because it fixes #23081 |
stable9: #24950 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Jenkins PR for #23344
Code-style changes added.