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

add option to check allowed_groups with the configured ldap search user #207

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tobi45
Copy link

@tobi45 tobi45 commented Jan 27, 2022

Summary

This pull request adds the boolean option use_search_user_to_check_groups which allows to switch the ldap user that is used to verify the membership of the user being authenticated with the allowed_groups. Its default value is False so that the behavior of the ldap authenticator is not changed.

Thus, if use_search_user_to_check_groups is:

  • False: the user being authenticated is used to check if she/he is member of one of the allowed_groups (current behavior)
  • True: the configured search user is used to check if the user being authenticated is member of one of the allowed_groups

This PR adresses #183.

Addressed Behavior

The plugin uses a so called search user to lookup the dn of the user to be authenticated. By doing so one connection is established to the ldap server. The authentication is done using an ldap bind which creates another connection to the server. Thus two connections to the ldap server are established with two different users: the search user and the user being authenticated.

Current Behavior

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 the allowed_groups is performed with the authenticated user instead of the search user.

Behavior with this PR

As the default value of the option use_search_user_to_check_groups is False the current behavior is not changed. If set to True the connection of the search user is used to check the allowed_groups for the user being authenticated.

Background

Our organization follows 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. The users just don't have the permission to look up the ldap groups. In such a setting only the configured ldap search user has such permissions.

Outline of Changes

  • add option use_search_user_to_check_groups
  • add parameter connection to method signature of resolve_username
  • moved the implementation to establish the connection with the search user from the method resolve_username into method authenticate to have the connection object with the search user available in method authenticate
  • add some comments in authenticate to outline whats going on
  • use either the object connectIon_user or connection_search to separate the connections of both users

Unfortunately I am not an ldap admin and couldn't provide an ldap server setup with such permissions set on an ldap test server. Instead to test the introduced ldap authenticator option I opted to mock the ldap connection objects and check if they are called appropriately.

I can confirm that it works with juypterhub 2.1.1.

tobias haubold added 2 commits January 26, 2022 17:12
…f the search user instead of the ldap connection of the user being authenticated, see jupyterhub#183
… pr-183-use-search-user-to-check-allowed-groups
@consideRatio
Copy link
Member

Thanks for an excellent description of why this is relevant @tobi45!

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

Successfully merging this pull request may close these issues.

2 participants