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

allowed_groups are looked up with authenticated user instead of search user #183

Open
tobi45 opened this issue Nov 5, 2020 · 7 comments
Labels

Comments

@tobi45
Copy link

tobi45 commented Nov 5, 2020

Bug description

The plugin uses a so called search user to lookup the dn of the user to be authenticated. The authentication is done using an ldap bind which creates another connection to the server. All subsequent ldap searches are performed with the connection of the authenticated user and not with the connection of the configured search user. Thus, the ldap query to check each of the allowed_groups is performed with the authenticated user instead of the search user.

The real problem behind this behavior: the authenticated user needs the ldap permissions to lookup the ldap groups. I think it's more the role of the search user to have such permissions instead of the authenticated user. Our institution follows such a consequent security approach where ldap groups are used for authorization by member check. But there is no need that the groups are itself accessible by the members.

Expected behaviour

I would expect that all search operations use the connection established with the search user.

Actual behaviour

After successful authentication, all ldap operations are done with the authenticated user instead of the search user.

How to reproduce

A source review targeting the following aspects or any working setup with debugger or debug output reveals this behavior.

method authenticate:
- calls resolve_username to use the search user to lookup the dn of the user to be authenticated
- later the variable conn is initialized with a get_connection method call with the user to be authenticated
- search_filter and allowed_groups queries use the connection (conn) with the successfully authenticated user

Your personal set up

  • Jupyterhub 1.2.1 with ldap authenticator (customized the official docker image)

How to proceed

I feel confident to fix this issue in a non intrusive way of keeping both connections to the ldap server and introduce a config flag which could be switched on to use the connection with the search user instead of the user to be authenticated.

Is such a contribution desired by the project? Or is the actual behavior the one which is expected by the project? I just ask as there are some pull request open from two years back.

@tobi45 tobi45 added the bug label Nov 5, 2020
@welcome
Copy link

welcome bot commented Nov 5, 2020

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@manics
Copy link
Member

manics commented Nov 5, 2020

@tobi45 Thanks for opening this issue! One of the problems with this authenticator is that it's difficult for the jupyterhub maintainers to review and test PRs, as many of the features are dependent on institutional LDAP and AD systems.

The logic you describe makes sense. If your PR minimise changes to existing code (so it's easy to review by inspection) and includes thorough tests it should be possible for us to review it subject to people having time. If you're interested in helping to keep this project running you can also help out by reviewing other PRs- this applies to all jupyterhub projects, you don't need to be an official maintainer to test and review PRs or triage issues 😀

@tobi45
Copy link
Author

tobi45 commented Nov 6, 2020

@manics Thanks for your quick answer! I totally understand that situation. And it sounds promising to me. Let's start with that issue and see how things evolve 😀

@tobi45
Copy link
Author

tobi45 commented Jan 25, 2021

Hey guys, some time ago I opened the pull request #185 which addresses this issue. I just want to ask if I could be of any help here? Don't hesitate. If it would help, I could open separate pull requests for the two things I've done in #185.

tobi45 pushed a commit to tobi45/ldapauthenticator that referenced this issue Jan 26, 2022
…f the search user instead of the ldap connection of the user being authenticated, see jupyterhub#183
@tobi45
Copy link
Author

tobi45 commented Jan 27, 2022

Hey guys, just to prevent confusion: I closed PR #185 and opened its replacement #207. Background: I throw the old repo fork away and created a new one with a new structure allowing me to manage my changesets better.

Please let me know If I could be of any help.

@petrrr
Copy link

petrrr commented Apr 19, 2023

Hi folks!
We are running into the same problem with our organizations LDAP server.

We have no access to groups by the authenticating user. We have an operator account (search user) which has access to groups and provides the expected responses. So this issue and the related PR #207 seems to address exactly our situation.

Any change the PR is merged and released any soon?
I do not see any way to get this solve otherwise.

@tobi45
Copy link
Author

tobi45 commented Apr 24, 2023

@petrrr it looks to me that the project is short on maintainers, thus I think rather not, unfortunately. :-/

I bypass the situation with my fork https://github.com/tobi45/ldapauthenticator, it contains only this changeset and provides a pip package with version 1.3.2.post1, which is version 1.3.2 of the ldapauthenticator with this changeset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants