#267 get all groups for ms-ad

strongloop / loopback-component-passport

LoopBack passport integration to support third party logins and account linking

Other

139 stars 228 forks source link

#267 get all groups for ms-ad #272

Closed Tyler-RCSD closed 5 years ago

Tyler-RCSD commented 5 years ago

Description

groupSearch results were based on the _groups ldap attribute and ignored the memberOf attribute which is traditionally used in Active Directory. This fix makes sure both work.

Related issues

connect to #267

Checklist

[x] New tests added or existing tests modified to cover all changes
[x] Code conforms with the style guide

slnode commented 5 years ago

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

ermakovov commented 5 years ago

@virkt25, @jannyHou, @b-admike, @dhmlau, hi!

Can you approve this pull request? This is very important for our current project!

jannyHou commented 5 years ago

@slnode test please

Tyler-RCSD commented 5 years ago

memberOf is the standard attribute within Microsoft Active Directory for groups. I think anyone would expect that this should work out of the box with AD without having to configure a custom ldapAttrName for groups.

I will see if I can figure out a test case next week. Honestly, I'm not all that familiar with writing test cases so if someone has a suggestion please let me know.

jannyHou commented 5 years ago

@Tyler-RCSD Got it. I quickly checked the test for configurator, adding a new one should be simple, you can follow this test case to add a similar one for memberOf 🙂

Tyler-RCSD commented 5 years ago

@jannyHou Can you review and see if I've done this properly?

jannyHou commented 5 years ago

@slnode test please

Tyler-RCSD commented 5 years ago

Thanks @jannyHou . How often do versions get released and pushed to npm?

jannyHou commented 5 years ago

@Tyler-RCSD Released 3.11.0 for it 🙂