Skip to content
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

Fixed failure when LDAP does not supply profile.memberOf and/or groups are queried separately. #254

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mussmanj
Copy link

Description

The connector crashes in users.js if profile.memberOf is undefined. And, if the groups provided are already in an array, it inappropriately nests that array in the "mail" array in profileMapper.js. The work was done in the branch "fixgroups" in my fork, I left it there for review and merging on the master side.

These two problems were discovered when during a CIC Auth0 class delivery for Okta training where for a demonstration I was trying to link to an Okta LDAP Interface which does not return "memberOf" with the user information and requires the group membership to be queried separately.

Testing

Test with an LDAP server that does not return "memberOf". This was discovered using the connector against the Okta LDAP interface, which does not return the groups with the user. Test the groups against an LDAP server where the group membership has to be queried separately, like the Okta LDAP interface.

Unit/integration tests for these two cases do not appear to be present in the test suite, but I did not create them. I will leave the missing tests to the maintainers.

Checklist

  • [ N/A] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • [ N/A] All active GitHub checks for tests, formatting, and security are passing
  • [ X] The correct base branch is being used, if not the default branch

@mussmanj mussmanj requested a review from a team as a code owner May 17, 2024 20:38
@mussmanj mussmanj requested review from gausnes and lostinauth0 May 17, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants