feat(ldap-auth): make querying user groups possible#2585
Conversation
729b731 to
73195d3
Compare
|
/ptal @nitrosx |
fpotier
left a comment
There was a problem hiding this comment.
I tried the feature with scicatlive but couldn't make it work out of the box.
It's maybe because we use a simplified ldap provider.
Left a few comments but I'm not sure it's worth implementing as you seem to be the only facility using this.
eec086f to
ef58a5f
Compare
add config options and use correct accessGroupProperty and add group extracting service Change-Id: Iddf3f95201fb2c7915fee5b0483afdb6232902e8
Change-Id: If1930b0a94e9c8cc14d2dbee629e2f534363fcaf
'_groups' in the ldap payload is from the library and won't change unless that is switched out. The field in the ldap response which represents the group name can vary however, so make that the configurable one (could e.g. be 'cn' or 'ou').
ef58a5f to
675081f
Compare
|
Can we merge it or is there any blocker for this to be merged? |
|
I missed that the environment variables were also in |
|
Besides making the ldap attribute for determining the group name configurable we also need this for determining the username. For non-ad setups e.g. using Imho we can do it as a small addition within this PR. If you want to have it separate please let us know. |
It looks find to me to have them in this PR. This PR is quite small and the implementation is straightforward, I'm happy for it to be merged |
The ldap authenticator did not query the groups from ldap until now.
Add the required config options for getting the groups from ldap. For this we just expose the existing options from node-ldapauth-fork/passport-ldapauth from the SciCat config.
Add a group extraction service for the LDAP response and register it to the servicefactory if enabled.
Additional fixes:
The
AccessGroupFromPayloadServicedid not correctly check whether the elements inpayload[accessGroupsProperty]are strings.