More LDAP auth patches #1669
Merged
More LDAP auth patches #1669
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove code duplication by factoring out the calculation of the LDAP query attributes out of _login2() resp. _login3() into __init__().
Ask for the 'memberOf' attribute to be returned in the user query only if 'ldap_load_groups' is set to True. This fixes the issue that currently LDAP authentication can only be used on LDAP servers that know this non-standard (it's an Active Directory extension) attribute. Other LDAP servers either do not necessarily have the group memberships stored in the user object (e.g. OpenLDAP), or use different attributes for this purpose (e.g. Novell eDirectory uses 'groupMembership')
pbiering
reviewed
Jan 3, 2025
DOCUMENTATION.md
Outdated
| @@ -932,14 +932,20 @@ The LDAP attribute whose value shall be used as the user name after successful a | |||
|
|
|||
| Default: not set, i.e. the login name given is used directly. | |||
|
|
|||
| ##### ldap_load_groups | |||
| #### ldap_groups_attribute | |||
There was a problem hiding this comment.
should have 5x #, please fix also current buggy entry:
#### ldap_user_attribute
There was a problem hiding this comment.
Please also extend the changelog
There was a problem hiding this comment.
done - thanks for the cross-check
This attribute is supposed to hold the group membership information if the config option 'ldap_load_groups' is True. If not given, it defaults to 'memberOf' for Active Directory. Introducing this options allows one to use radicale's LDAP auth with groups even on LDAP servers that keep their group memberships in a different attribute than 'memberOf', e.g. Novell eDirectory which uses 'groupMembership'.
The same effect can be achieved using the option 'ldap_groups_attribute' alone, if it's default becomes unset instead of 'memberOf' Benefit: one config option less to deal with. While at it, also fix header level for 'ldap_user_attribute' in documentation.
Use helper methods from the LDAP modules to get individual elements (like in our case the RDN value) out of attributes with DN syntax in a standard compliant way instead fiddling around ourselves. If these methods fail, fall back to using the whole attribute value, which allows us to also use attributes with non-DN syntax for groups and permissions.
marschap
force-pushed
the
more-LDAPauth-patches
branch
from
2c297c7 to
d6c4e64
Compare
|
Please add changelog entry related to changed option |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
here's another round of LDAP auth patches
They deal with issues not dealt with in the previous round:
avoid code duplication by calculating attributes to query in
__init__()allow using LDAP authentication also to servers that do not know the
memberOfattributeby querying it only if
ldap_load_groupsisTrueintroduce config option
ldap_groups_attributethat allows us to configure the groups attribute nameinstead of having it being hard coded to
memberOfget rid of
ldap_load_groupsconfig option. - havingldap_groups_attributebeing unsetresp. being set to a specific attribute name has the exact same effect
stop fiddling around with getting the RDN value of the groups attributes returned,
and instead use the LDAP modules' helper functions foreseen for this purpose.
In addition, this gives us a bit more flexibility with the
ldap_groups_attribute: we can use attribute with non-DN syntax too.As radicale does not rely on on the DN syntax, but only takes the RDN value anyway, this is no limitation, but a real extension
Please consider incorporating them into mainline radicale