-
Notifications
You must be signed in to change notification settings - Fork 178
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
Lookup additional LDAP user info #103
Conversation
This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there: |
Just FYI, I made it here from jupyterhub/kubespawner#298 and this is exactly what we needed for deployment in OpenShift |
@minrk any possibility of this getting reviewed/merged at this point? The community has worked around it up to this point by copy-pasting a customized |
@yuvipanda @dhirschfeld anyone maintaining this repo? How can we get this moved forward? Seems like a simple feature, and a copy-pasted version of this code is already used in production by many in the community |
I have literally zero time to work on this repo and although I have commit rights I don't really consider myself a maintainer. I could merge this but because this repo doesn't have any tests it might break for other people and I wouldn't have the time to fix it (except possibly revert). Also, unlike most users I'm on Windows so I wouldn't even be able to test locally against the setup most people have So, unfortunately development is at a bit of a standstill and if people need additional features they have to effectively deploy from a fork which isn't an ideal place to be. If someone wanted to figure out and contribute a way to test existing/new features I would be ok stepping up to merge new PRs but failing that I probably won't do so until I can carve out time to look at this properly - which won't be in the immediate future. |
Thanks for clarifying the situation Dave Running from a fork is causing my company some headaches - I might be able to negotiate time to contribute a CI setup for this repo It looks like the main JupyterHub repo uses Travis - I could set up something similar and write some simple tests for the existing functionality |
@dhirschfeld I opened a PR with a Travis lint + test setup and managed to get 80% coverage with some simple tests based on the auth tests in the main JupyterHub repo: #134 If we were to get that reviewed and merged, and then I add a test case for the feature in this PR, would you be comfortable merging it? |
@marcusianlevine Thanks for #134 ! I'll look into rebasing this PR and adding a test, I'll ping you for help if I can't figure out |
I've merged in master and added a test for this feature. |
No one disagreed so I've renamed |
@manics - I'll take a proper look Monday 12th. FWIW it looks good to me so I've "approved" the changes in case anyone wants to merge before then... |
@dhirschfeld think we could cut a new release once we get this merged? |
Any update on when this might be merged? |
@manics - Is it possible to please share a version of ldapauthenticator.py with these changes for the one used for jupyterhub v0.7.0
|
@ramkrishnan8994 I guess this is related to jupyterhub/zero-to-jupyterhub-k8s#1402 ? Have you tried using this version of LDAPAuthenticator with the old Z2JH chart? If it doesn't work you'll either have to port these changes, or upgrade your Z2JH version. |
That issue has been solved. It isn't exactly related. The only relation is that there are some dependency issues that are stopping us from deploying the new version of Jhub. I'm trying to use the ldapauthenticator.py from the MR to v0.7.0. But that has its own challenges. Also the documentation in - https://gist.github.com/manics/c4bcf53a210d444db9e64db7673e8580 |
There any reason not to merge this? We would really like to use zero to k8s with ldap based uids and a shared home. This seems to be a prereq? |
@jupyterhub/jupyterhubteam can we decide how to take this forward? It seems to be generally useful. |
Hi guys, |
I think we need more maintainers for this repo! I don't know who has LDAP experience in the Jupyter world... I've just merged this. Would a release be useful? |
Thanks Yuvi, a release would be great! 🙌 I'd be happy to volunteer to become a maintainer. I wrote the test suite and CI setup for this repo, and I use it in production at my company. |
How would I tell jupyterhub to use a specific release of ldapauthenticator ? |
@titansmc, where jupyterhub runs, the specific release of ldapauthenticator that you want to use should be installed. But where does it run, how does it run, etc - it depends on how you have installed jupyterhub. Is it by a custom installation, a zero-to-jupyterhub-k8s installation, or the-littelest-jupyterhub installation etc. Please open a new issue to discuss this where you think is most suitable. |
This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there: https://discourse.jupyter.org/t/ldap-uid-gid-integration-with-jupyterhub-k8s-hub-0-9-0-beta-2/3095/2 |
Adds a new property
user_info_attributes
that lists additional LDAP fields to lookup and return inauth_state
for a valid user.Example use: Run a containerised singleuser server as the numeric user ID from LDAP so that files created on a shared external mount have the expected owner ID.