Add slightly better Active Directory support #94

jfautley · 2018-07-12T16:05:16Z

This change does the following:

Allows looking up group membership from the LDAP memberOf attribute attached to the user's LDAP object
Provides Active Directory specific lookup support to resolve nested groups (EDIT: Fixes Members of nested groups aren't allowed if only the parent group is listed under allowed_groups #177)
Restores previous behaviour by allowing bind_dn_template to be provided with the user's provided login name rather than just the resolved version (EDIT: Relates to Active Directory requires "optional" bind_dn_template #146 perhaps?)

It is anticipated this makes using JupyterHub when authenticating against Microsoft AD somewhat easier.

jfautley · 2018-08-22T12:46:16Z

bump

Is this something you're interested in (in which case I'll work on rebasing against master) or is there a better way to handle binding to AD?

dhirschfeld · 2018-08-23T04:53:21Z

Unfortunately I've got zero bandwidth to look at this just at the moment. As I'm using Active Directory I do plan on looking at this when time allows but for me at least that's measured in weeks rather than days...

dhirschfeld · 2018-08-23T04:56:31Z

ldapauthenticator/ldapauthenticator.py

@@ -62,6 +63,9 @@ def _server_port_default(self):
            	uid={username},ou=people,dc=wikimedia,dc=org,
            	uid={username},ou=Developers,dc=wikimedia,dc=org
        	]
+
+        Active Directory Example:
+            DOMAIN\{login}


I'm curious why this is needed? I'm on AD and I login with my samid (DOMAIN\samid) which is used to look up my CN which is then substituted into my DN template.

For people with weird and wonderful OU configurations setting templates in bind_dn is either very complex or potentially impossible (especially if you don't directly control Active Directory and the admin team keep making changes!). Being able to use standard AD syntax of DOMAIN\username to bind with makes life much easier and means the JH admin doesn't have to care about the backend AD structure.

dhirschfeld · 2018-08-23T05:02:50Z

ldapauthenticator/ldapauthenticator.py

+        """
+    )
+
+    activedirectory = Bool(


It seems a bit of a code-smell to have both get_groups_from_user and activedirectory. I have a niggling feeling that there must be a better way to do this... but I'm not an expert so, maybe there isn't?

Having more configuration knobs to turn adds complexity and this is especially true if they're only valid in some situations and not others. Given that I'd want to make sure they were really necessary and the benefit outweighed the added complexity (and hence maintenance burden)

The reason for the different configuration setting is to ensure we can use the AD specific search functionality to look up users in nested groups. AD user objects only have groups the user is a direct member of as a memberOf attribute on their object.

For example, if we have group 'JH Users' which contains 'Lab APAC' and 'Lab EMEA' groups, a user in 'Lab EMEA' won't also show up as being in 'JH Users' when you query the memberOf attribute on the user object, so you need to use the AD specific search syntax.

There is not, as far as I know, an easy way to tell if you're talking to 'Active Directory' LDAP or another implementation hence the configuration option. An alternative might be to test the AD specific search and if it fails, fall back to the "standard" behaviour but to me this seems to smell even more than the 'duplicate' code.

sigurdurb · 2018-10-16T12:46:02Z

You know you can authenicate against groups with memberOf by just adding it to the lookup_dn_search_filter
Here is an example:

c.LDAPAuthenticator.lookup_dn_search_filter = '(&({login_attr}={login})(memberOf=CN=SomeCnName,OU=SomeOu,OU=SomeOU,DC=bla,DC=com))'

and also using c.LDAPAuthenticator.user_attribute = 'sAMAccountName' that maps to the {login_attr} in the variable above.

myoung34 · 2018-10-25T17:11:36Z

we really need this.
Right now the way it works is only by a specific OU string which is terrible.

Group membership would be much better

jfautley · 2018-10-30T10:26:51Z

Seems like there's at least one other person who would find this useful so I'll have a go at rebasing the changes to master.

dhirschfeld · 2018-10-30T10:57:41Z

Does the memberOf trick mentioned by @sigurdurb not work for you?

jfautley · 2018-10-30T11:01:25Z

Does the memberOf trick mentioned by @sigurdurb not work for you?

I don't see how that actually fixes the underlying issue - it would only work for a single group, and still relies on the memberOf attribute which isn't populated for nested groups in AD.

jfautley · 2018-10-30T14:04:01Z

I've reworked the logic here a bit, and twiddled some config options which hopefully make it a bit less finicky to configure, and simplify both AD and non-AD configurations and make it clear which is which.

dhirschfeld · 2018-11-06T23:46:57Z

I'm pretty swamped (and don't really consider myself a proper maintainer) but I'll try to set aside some time to take a look...

jfautley · 2019-04-24T08:23:19Z

Just a gentle reminder here - it would be nice to get this merged (or rejected so I can work on an alternative if this PR isn't suitable).

Active Directory (for better or worse...) is very popular in enterprise environments and this PR provides better integration with the Microsoft-way of managing LDAP.

If you don't want the code-smell of if ad: <code> else: <almost the same code> then I'm also happy to fork the repo and maintain a specific Active Directory authenticator for JupyterHub but I'd (personally) rather keep the code integrated (also I don't really want to maintain a fork of an already solid codebase!).

Also add Microsoft Active directory specific lookup logic to resolve nested groups.

See jupyterhub#171 for further information

See cannatag/ldap3#855 (comment)

…221615a64a506529c775abe593/.github/workflows

The port was changed at rroemhild/docker-test-openldap@adb4650

Co-authored-by: Simon Li <[email protected]>

isaacsanders · 2022-06-22T19:00:41Z

ldapauthenticator/ldapauthenticator.py

+            if self.activedirectory:
+                self.log.debug('Active Directory enabled')
+                user_dn = self.resolve_username(username)
+                search_filter='(member:1.2.840.113556.1.4.1941:={dn})'.format(dn=escape_filter_chars(user_dn))


1.2.840.113556.1.4.1941 feels like a magic string, would extracting it as a well known constant be acceptable to you? https://ldapwiki.com/wiki/1.2.840.113556.1.4.1941

consideRatio · 2024-09-16T22:30:35Z

Despite "slightly better" in the title, this looks like a lot of great work! Thank you @jfautley for the effort into this and everyone involved in review efforts!

I've recently onboarded myself as a maintainer to this project, and I think the right call for this PR is to close it at this point.

I note that:

Being able to configure group attributes is now possible thanks to Allow users to configure group search filter and attributes (group_search_filter and group_attributes config) #168
Handling nested groups is still not supported and tracked via Members of nested groups aren't allowed if only the parent group is listed under allowed_groups #177, logic in this PR may be still be valuable to extract to resolve that

dhirschfeld reviewed Aug 23, 2018

View reviewed changes

jfautley force-pushed the fix-ad-binding branch from f17c03e to dfe1788 Compare October 30, 2018 14:03

jfautley force-pushed the fix-ad-binding branch 2 times, most recently from ef6c7e0 to b90fab3 Compare July 30, 2019 12:39

Allow lookup of groups directly from user object

1732ad1

Also add Microsoft Active directory specific lookup logic to resolve nested groups.

jfautley force-pushed the fix-ad-binding branch from b90fab3 to 1732ad1 Compare March 5, 2020 18:34

Aethylred mentioned this pull request Sep 10, 2020

Members of nested groups aren't allowed if only the parent group is listed under allowed_groups #177

Open

1kastner and others added 12 commits March 31, 2022 16:56

Pin ldap3 version to lower than 2.8

c50ac68

See jupyterhub#171 for further information

1.3.1 changelog

c04ce4f

release 1.3.1

3e46906

back to dev

506caef

Exchanging ldap3 constants in if/else

1a61314

See cannatag/ldap3#855 (comment)

Update ldapauthenticator.py

0c11004

Remove fixed version

73023f8

1.3.2 changelog

121b049

release 1.3.2

bb82d23

back to dev

caaf7ad

Copy https://github.com/jupyterhub/oauthenticator/tree/be91e7d9e58f13…

b42218a

…221615a64a506529c775abe593/.github/workflows

ldapauthenticator GitHub test workflow

c806dfd

manics and others added 6 commits March 31, 2022 16:56

Remove travis

d1eeacc

Fix traitlets warnings when running tests

8cdf3e9

Change test ldap server port mapping

4675773

The port was changed at rroemhild/docker-test-openldap@adb4650

Add study participation noteice to readme

b45ac31

Update README.md

b1dbcbf

Co-authored-by: Simon Li <[email protected]>

Merge branch 'jupyterhub:master' into fix-ad-binding

08f2ad5

isaacsanders reviewed Jun 22, 2022

View reviewed changes

consideRatio mentioned this pull request Jun 4, 2023

Release plan for 2.0.0 #220

Closed

12 tasks

consideRatio closed this Sep 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add slightly better Active Directory support #94

Add slightly better Active Directory support #94

jfautley commented Jul 12, 2018 •

edited by consideRatio

Loading

jfautley commented Aug 22, 2018

dhirschfeld commented Aug 23, 2018

dhirschfeld Aug 23, 2018

jfautley Oct 30, 2018

dhirschfeld Aug 23, 2018

jfautley Oct 30, 2018

sigurdurb commented Oct 16, 2018 •

edited

Loading

myoung34 commented Oct 25, 2018

jfautley commented Oct 30, 2018

dhirschfeld commented Oct 30, 2018

jfautley commented Oct 30, 2018

jfautley commented Oct 30, 2018

dhirschfeld commented Nov 6, 2018

jfautley commented Apr 24, 2019

isaacsanders Jun 22, 2022

consideRatio commented Sep 16, 2024

Add slightly better Active Directory support #94

Add slightly better Active Directory support #94

Conversation

jfautley commented Jul 12, 2018 • edited by consideRatio Loading

jfautley commented Aug 22, 2018

dhirschfeld commented Aug 23, 2018

dhirschfeld Aug 23, 2018

Choose a reason for hiding this comment

jfautley Oct 30, 2018

Choose a reason for hiding this comment

dhirschfeld Aug 23, 2018

Choose a reason for hiding this comment

jfautley Oct 30, 2018

Choose a reason for hiding this comment

sigurdurb commented Oct 16, 2018 • edited Loading

myoung34 commented Oct 25, 2018

jfautley commented Oct 30, 2018

dhirschfeld commented Oct 30, 2018

jfautley commented Oct 30, 2018

jfautley commented Oct 30, 2018

dhirschfeld commented Nov 6, 2018

jfautley commented Apr 24, 2019

isaacsanders Jun 22, 2022

Choose a reason for hiding this comment

consideRatio commented Sep 16, 2024

jfautley commented Jul 12, 2018 •

edited by consideRatio

Loading

sigurdurb commented Oct 16, 2018 •

edited

Loading