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

[management-api] [ldap] Full name of LDAP users not shown if LDAP object lacks givenname/sn #1030

Closed
hfuru opened this Issue Jan 23, 2018 · 11 comments

Comments

Projects
None yet
2 participants
@hfuru

hfuru commented Jan 23, 2018

I can find a user by his name, but Gravitee just shows his username, not the full name it just found, because our LDAP objects do not contain the attributes Gravitee expects (givenName, sn).

(We use a patch which doesn't search persons at all, but accounts, hence the missing sn).

Expected Behavior

Show the LDAP user's full name (cn) if other attrs are missing.

Current Behavior

Showing username but not full name.

Possible Solution

LdapIdentityLookup.java (gravitee-management-rest-api) searches for a person by his CN, and then tries to show his givenName, sn and mail, as far as I can see from the code. However, givenName and mail are not part of LDAP's 'person' objectclass. For that matter, LDAP servers can use access controls to hide some attributes, and Gravitee does not look up the 'initials' attribute which could complete a person's name.

givenName/initials/mail are commonly from the inetOrgPerson object class, that is likely the class hwich the code is expecting. But it also has a displayName attribute which is the preferred form to show the name. Other variants are cn, or compose givenName+initials+sn. However, remember that the order of these attributes vary between cultures, so there is no trivial and correct way to "translate" between the 3-attr form and 1-full-attr form.

Steps to Reproduce (for bugs)

  1. In apim management:gravitee.yml, refer {security:providers: type ldap} to an LDAP directory where ldap users have cn but not givenName (and sn, if you have that patch).
  2. On the web api, find an LDAP user by his full name (cn).
  3. Gravitee shows the username - and maybe sn if it is a person, I have not tested that.

More or less, anyway. I don't have a working installation to test this on right now.

Context

Authenticate users in LDAP and have them shown in a useful way.

Your Environment

  • Version used: Gravitee 1.12.2 + #948
  • Browser Name and version: Firefox
  • Operating System and version: Red Hat Enterprise Linux Server release 7.4 (Maipo)
@brasseld

This comment has been minimized.

Member

brasseld commented Jan 23, 2018

As far as I understand, you need a way to customize the mapping between LDAP attributes and Gravitee user fields, am I right ?

I mean this part from source code: https://github.com/gravitee-io/gravitee-management-rest-api/blob/master/gravitee-management-api-idp/gravitee-management-api-idp-ldap/src/main/java/io/gravitee/management/idp/ldap/lookup/LdapIdentityLookup.java#L137

@hfuru

This comment has been minimized.

hfuru commented Jan 24, 2018

Yes, that would be preferable. But more importantly, the default behavior is wrong. You need a minimal fallback which should be either to display the same attr which was searched for, or just display "cn" since that is the attr which will most commonly be available. Once you have that fallback, you can try something more fancy like showing displayName if it is there, or showing givenName+initials+sn if you find givenName+sn (similar to what you do now).

@brasseld

This comment has been minimized.

Member

brasseld commented Jan 24, 2018

Ok so you agree that we are speaking about the search functionality and not the authentication one ? In that case, it is easier for us to apply such changes.

@hfuru

This comment has been minimized.

hfuru commented Jan 24, 2018

@hfuru

This comment has been minimized.

hfuru commented Jan 24, 2018

@brasseld

This comment has been minimized.

Member

brasseld commented Feb 5, 2018

I had a deeper look on this issue.

I don't understand the adminLimitExceeded LDAP error because we have set the number of expected results in the LDAP query. See:
https://github.com/gravitee-io/gravitee-management-rest-api/blob/master/gravitee-management-api-idp/gravitee-management-api-idp-ldap/src/main/java/io/gravitee/management/idp/ldap/lookup/LdapIdentityLookup.java#L98

I'm having a look to the other points.

@hfuru

This comment has been minimized.

hfuru commented Feb 5, 2018

@hfuru

This comment has been minimized.

hfuru commented Feb 5, 2018

@brasseld

This comment has been minimized.

Member

brasseld commented Feb 5, 2018

Yes please fill an other issue.
Let's keep this one for query / filter rules and attributes used to display users.
I let you create an other issue to manage the adminLimitExceeded error.

@brasseld

This comment has been minimized.

Member

brasseld commented Feb 12, 2018

Will be part of #1053

@brasseld brasseld changed the title from Full name of LDAP users not shown if LDAP object lacks givenname/sn to [management-api] [ldap] Full name of LDAP users not shown if LDAP object lacks givenname/sn Feb 13, 2018

@brasseld brasseld added this to the 1.14.0 milestone Feb 13, 2018

@brasseld brasseld self-assigned this Feb 13, 2018

@brasseld

This comment has been minimized.

Member

brasseld commented Feb 20, 2018

Closed by #1053

@brasseld brasseld closed this Feb 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment