mod_shared_roster_ldap ignores ldap_uids

huhndev commented 5 months ago

Environment

ejabberd version: 21.12-1
Erlang version: Erlang (SMP,ASYNC_THREAS) (BEAM) emulator version 12.2.1
OS: Linux (Ubuntu 22.04)
Installed from: distro package

Configuration:

  mod_shared_roster_ldap:
    ldap_auth_check: false
    use_cache: false
    ldap_base: "cn=accounts,dc=example,dc=com"
    ldap_filter: "(|(objectClass=posixAccount)(objectClass=posixGroup))"
    ldap_rfilter: "(&(objectClass=posixGroup)(cn=worker))"
    ldap_gfilter: "(&(objectClass=posixGroup)(cn=%g))"
    ldap_groupattr: "cn"
    ldap_groupdesc: "cn"
    ldap_memberattr: "member"
    ldap_memberattr_format: "uid=%u,cn=users,cn=accounts,dc=example,dc=com"
    ldap_ufilter: "(&(objectClass=posixAccount)(!(nsaccountlock=true))(uid=%u))"
    ldap_useruid: "uid"
    ldap_userdesc: "cn"
    ldap_uids:
      - pager

Errors from error.log/crash.log

No errors

Bug description

The usage of ldap_uids doesn't change the behavior of mod_shared_roster_ldap. Always the same composition of uid and hostname (uid@example.com) gets into the roster of the users. Instead, I would assume the jid would be a composition of pager and hostname (pager@example.com).

pager looks like givenname.familyname while uid looks like givenname + x if this is relevant anyhow. ldap_uids is set in mod_vcard and the top-level too, without any problems.

Any ideas why ldap_uids gets ignored by mod_shared_roster_ldap?

licaon-kter commented 5 months ago

duplicate of https://github.com/processone/ejabberd/issues/4029 ?

prefiks commented 5 months ago

So, ldap_uids is not used in shared_roster_ldap at all, from what i see only things that are affecting jid of contacts in generated roster entries are ldap_memberattr (which will be used to together with hostname) and ldap_userjidattr which value will be parsed as full jid.

huhndev commented 5 months ago

The docs show ldap_uids as a valid option for mod_shared_roster_ldap. Anyway, there seems something broken. If I set ldap_userjidattr as mail attribute of my ldap tree, then ldap_userdesc gets ignored.

badlop commented 5 months ago

You both are right:

The documentation for the option ldap_uids in mod_shared_roster_ldap was added in https://github.com/processone/ejabberd/commit/a02cff0e780bb735531594c4ece81e8628f79782
However, previously to that commit, that option was not documented or implemented in that module. And that commit did not implement that option: it was focused just on switching documentation format in many many modules.

In summary: the option was never implemented or even planed, the documentation was mistakenly added when copying text from other modules to that one.

The documentation should get fixed: check if there are any other options that get also added by mistake, and remove mention to inexistent options.

And the next question is: is it worth implementing ldap_uids in mod_shared_roster_ldap, how hard would it be, when and who? And if it's done, then the documentation is added.

huhndev commented 5 months ago

Many thanks for the clarification, @badlop.

As a workaround I have now set ldap_userjidattr and try to set the common name/display name with ldap_userdesc. Unfortunately this does not work either and display name is givenname.familyname, as in the user part of the mail attribute, instead of Givenname Familyname as in cn. At least the JID is now correct. Any idea why this is happening? Should this work at all? That's how it's described in the documentation.

Below is the modified configuration of which the last two lines are of interest:

  mod_shared_roster_ldap:
    ldap_auth_check: false
    use_cache: false
    ldap_base: "cn=accounts,dc=example,dc=com"
    ldap_filter: "(|(objectClass=posixAccount)(objectClass=posixGroup))"
    ldap_rfilter: "(&(objectClass=posixGroup)(cn=worker))"
    ldap_gfilter: "(&(objectClass=posixGroup)(cn=%g))"
    ldap_groupattr: "cn"
    ldap_groupdesc: "cn"
    ldap_memberattr: "member"
    ldap_memberattr_format: "uid=%u,cn=users,cn=accounts,dc=example,dc=com"
    ldap_ufilter: "(&(objectClass=posixAccount)(!(nsaccountlock=true))(uid=%u))"
    ldap_useruid: "uid"
    ldap_userjidattr: "mail"
    ldap_userdesc: "cn"

huhndev commented 4 months ago

@badlop, @prefiks: Do you have any feedback on the bug I described in my last message?

ldap_userdesc has no effect if ldap_userjidattr is set.

Neustradamus commented 4 months ago

The best LDAP external dev who has contributed in ejabberd is @ethoms. Maybe he can help on this ticket...

huhndev commented 4 months ago

Reproducible with ejabberd 23.10-1.

huhndev commented 4 months ago

Reproducible with ejabberd 24.02.

processone / ejabberd