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

Added backend support for LDAP group lookups from the user #3936

Conversation

shawnweeks
Copy link

Details

  • Does this resolve an issue?
    Resolves #

Changes

New Features

  • Adds feature which looks up a users group in LDAP from the user memberOf or related attribute

Breaking Changes

  • Should not be a breaking change as the default behavior has not changed.

Additional

This is just a preliminary PR and is not complete nor ready to merge.

@netlify
Copy link

netlify bot commented Nov 3, 2022

Deploy Preview for authentik ready!

Name Link
🔨 Latest commit 3279edb
🔍 Latest deploy log https://app.netlify.com/sites/authentik/deploys/6363d2d9456dd300093f7ac4
😎 Deploy Preview https://deploy-preview-3936--authentik.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@shawnweeks shawnweeks force-pushed the feature/source_ldap_lookup_groups_from_user branch from 3279edb to 09dc098 Compare November 4, 2022 12:39
@@ -34,7 +34,24 @@ def sync(self) -> int:
)
membership_count = 0
for group in groups:
members = group.get("attributes", {}).get(self._source.group_membership_field, [])
if self._source.lookup_groups_from_user:
group_dn = group.get("dn", {})
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed where you sometimes lookup an attribute called "distinguishedName" but you don't actually have to since the LDAP library your using always pulls the "dn" anyway. In Active Directory they always match as far as I can tell and "dn" is part of the LDAP spec.

@@ -53,7 +70,7 @@ def sync(self) -> int:
"ak_groups__in": [ak_group],
}
)
)
).distinct()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a distinct here because I noticed it was pulling back the same user multiple times if they were in multiple groups due to the join. It seemed wrong to call group save with the same user multiple times but Django may already be handling that internally.

@derlucas
Copy link

I'm very interested in this feature since i want to implement nested Groups in my AD Forest.
Is there any way i can help with the Integration of that feature?

@shawnweeks
Copy link
Author

@derlucas This will work against AD by setting the member of attribute to memberOf:1.2.840.113556.1.4.1941: as well and I'm testing it against both. I need to just sit down and get some test cases worked out and get some feedback. Hope to work on some test cases this weekend.

@stale
Copy link

stale bot commented Feb 20, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status/wontfix This will not be worked on label Feb 20, 2023
@stale stale bot closed this Feb 27, 2023
@Minionflo
Copy link

Any update?

@fdisamuel
Copy link

Hey! I would really love to see this feature PR merged as our company uses nested LDAP Groups heavily in their setup. Without being supported in Authentik, proper group sync from our AD server seems to be impossible...

@cybertschunk
Copy link

hey, I 'd like to continue working on the PR. Would that be possible? We really need support for nested group in our environment.

@fdisamuel
Copy link

Hello @cybertschunk, it's great to see your readiness to add this feature.😊 Concerning the current PR, unfortunately, it may not be feasible to rebase due to it being 4600 commits behind the latest authentik codebase release. It might be more practical to implement the nested sync functionality in a new Pull Request using the most recent source code (I think, as a non-programmer ) However I would be happy to hear a feedback from the Authentik Dev Team regarding this topic!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants