search

Crivaledaz / Mattermost-LDAP

This module provides an external LDAP authentication in Mattermost for the Team Edition (free).
MIT License
357 stars 71 forks source link

Return the displayName from LDAP so users have proper names #89

Closed gdevenyi closed 2 years ago

gdevenyi commented 2 years ago

Currently the oauth is returning "cn" for the name to Mattermost, which is not generally an attribute intended for display.

Active Directory and the RFC openldap schemas have displayName for this.

WanpengQian commented 2 years ago

displayName is not a MUST attribute against cn. it is better fallback to cn if displayName does not exist,

gdevenyi commented 2 years ago

image

WanpengQian commented 2 years ago

I am not saying displayName is illegal attribute or not a existing value. if you delele the displayName, it is ok. but if you delete cn, it can not be done. Snipaste_2022-05-20_09-41-06 Snipaste_2022-05-20_09-41-25 for general use, you cannot assume displayName attribute is always useable.

gdevenyi commented 2 years ago

for general use, you cannot assume displayName attribute is always useable.

For general use, every ldap implemetnation I've read about so far includes displayName as it is front and center in the three most popoular schemas.

WanpengQian commented 2 years ago

I am not saying displayName is not included( or defined) in the schemas. The fact is displayName is an optional attribute. for a record, it is totally legal without this attribute. the LDAP server will accept this record. With the change here, it will throw an error. You cannot assume an optional attribute must exists. against a MUST attribute(in this case, the cn attribute).

Crivaledaz commented 2 years ago

Thank you for your contribution.

I agree with @WanpengQian, we cannot assume the displayName attribute is always defined in a directory, as it is not a MUST attribute in common schemes. In fact, displayName belongs to the InetOrgPerson, it is not mandatory and does not need to be unique. Whereas cn is part of the Distinguish Name (DN), it must be defined and unique in the OU.

So we can assume the cn attribute always exists and it should be kept as a fallback if the displayName attribute is not defined. However, when the last one is defined, I agree Mattermost-LDAP should use it.

@WanpengQian implemented this in #92 and I have merged it few minutes ago. So you can access this feature by pulling the master branch.

Let me know if it fills your need, or if you have any feedback.

Since it is covered by #92, I close this pull request.