ldap auth groups memberOf queried even if no groups

[alien999999999]

in ldap there are 2 ways to make groups: either the groups have member or uniqueMember; or the user has memberOf or similar attributes.

radicale only has the memberOf method and this attribute is hardcoded (instead of config)

also, even if groups are configured off, this memberOf attribute is still queried.

and if your custom ldap schema does not have memberOf, (like mine) the PROPFIND of user will fail.

if i remove memberOf in the attribute list to be requested, it does work (without groups).

I would like it for both ways to work config item and to have the memberOf or uniqueMember to be configurable and most importantly, don't request memberOf if groups is configured off

specifically:

• ldap_group_direct = False
• ldap_group_indirect_attribute = "memberOf"
• ldap_group_direct_attribute = "member" # or uniqueMember
• ldap_group_direct_filter = "(&(cn={0})(objectClass=groupOfNames))" # or groupOfUniqueNames
• if not ldap_group_direct and ldap_groups: user_attributes.append(ldap_group_indirect_attribute)
• next to user lookup, also do a group lookup (if ldap_group_direct )
• (optionally: one could also not add these attributes if they were None)
• also, this group name could be used to give collection-shared/{0}/* (next to collection-root/{0}/* ) for easier sharing (in owner_write rights mode)

[pbiering]

@petervarkoly / @marschap - can you please take a look into?

[petervarkoly]

I start to implement a flexible solution to work with the groups.

[marschap]

To me the request looks like overkill for an edge case.

The request even start with false premises, which hint (at least to me) that the requester may not have installed the latest version, e.g.

• the ways to define groups in LDAP that he describes are no alternatives:
  Ususally 'memberof' is the user-side counterpart of the 'member' in the group.
  A memberOf without a corresponding group object is definitely not a group definition,
  but a simple attribute holding a DN.
• the attribute memberOf is not hard coded.
  As 'memberOfmay be called different in other LDAP implementations (like e.g. Novell eDirectory, where it is called groupMemnership) radicale implemented the attribute ldap_groups_attributein version 3.4.0 In additionldap_groups_attribute does not even need to be a DN-valued attribute. Every attribute can be used. This is possible, because for DN-valued ldap_groups_attribute only the RDN is considered.

Even LDAP servers that implement the LDAP RFCs w.r.t to group very strictly (like e.g. OpenLDAP) have modules to create & maintain the user-side attributes (a.k.a. radicale's ldap_groups_attribute) using modules.

I recommend to stick with the existing version.

[petervarkoly]

@marschap Can you have a look at:
#1791

[marschap]

@petervarkoly I added some comments to the MR.

As I wrote above, I consider the issue an edge-case and the solution that the reporter proposed an absolute overkill (especially when looking at the other issue w.r.t "needlessly querying the LDAP schema".

In addition the reporter seems to completely have missed the point of the ldap_groups_attribute, which I implemented to:

1. support other attributes for the user-side than the hard-coded AD-specific memberOf
   (e.g. Novell eDirectory servers, but also other implementations like OpenLDAP)
2. support non-DN valued attributes
   (possible because even for DN-valued attributes only the RDN value is used, i.e. you can use any attribute ;-))

Re your patch:

• it mixes the meaning of ldap_groups_attibute: It was meant to be an attribute of the user entry (to avoid performing additional group searches). With your patch it becomes sometimes a user-side attribute, sometimes a group-side attribute. This is absolutely confusing and only understandable when you read the code.
• it brings back hard-coded values (memberOf) to distinguish between cases.
  By doing so you break working use cases:
  • other LDAP server implementations that use other attributes than memberOf
  • DN & non-DN-valued use cases using other user-side attributes that do not correspond to groups.
    Currently you may use seeAlso on the user side, which is DN-valued, but also - like mentioned - e.g. carLicense.
    With your patch this will break.

I do not consider it acceptable to bring back hard coded values to distinguish between use cases and breaking exyisting use cases.

Please revert the MR
Sorry

[alien999999999]

To me the request looks like overkill for an edge case.

The request even start with false premises, which hint (at least to me) that the requester may not have installed the latest version, e.g.

* the ways to define groups in LDAP that he describes are no alternatives:
  Ususally 'memberof' is the user-side counterpart of the 'member' in the group.
  A `memberOf` without a corresponding group object is definitely not a group definition,
  but a simple attribute holding a DN.

* the attribute `memberOf` is not hard coded.
  As 'memberOf`may be called different in other LDAP implementations (like e.g. Novell eDirectory,  where it is called `groupMemnership`) radicale implemented the attribute `ldap_groups_attribute`in version 3.4.0 In addition`ldap_groups_attribute` does not even need to be a DN-valued attribute. Every attribute can be used. This is possible, because for DN-valued ldap_groups_attribute` only the RDN is considered.

Even LDAP servers that implement the LDAP RFCs w.r.t to group very strictly (like e.g. OpenLDAP) have modules to create & maintain the user-side attributes (a.k.a. radicale's ldap_groups_attribute) using modules.

I recommend to stick with the existing version.

• So, with LDAP you can make your own custom schema and there are 2 ways used to indicate groups, since ages ago.
• in the code i was looking at, ((perhaps i had an older version), it was hardcoded as an attribute to fetch while getting a user (irrespectively of groups being on or off), which broke for me, beause memberOf is not defined in my schema.

[pbiering]

any conclusion now?