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

LDAP user group/role checking #10

Closed
animedbz16 opened this issue Apr 22, 2014 · 11 comments
Closed

LDAP user group/role checking #10

animedbz16 opened this issue Apr 22, 2014 · 11 comments

Comments

@animedbz16
Copy link

It would be great if there was a way to pass additional options for a group DN, group filter in such a way that when you authenticate a user then lookup what groups that user is a member of.

Example:

{
  bindUserDN: "CN=bindUser,OU=Users,DC=domain,DC=com",
  bindUserPassword: "password",
  userBaseDN: "OU=Users,DC=domain,DC=com",
  userFilter: "(&(objectClass=person)(sAMAccountName={{username}}))"
  userAttributes: ['sAMAccountName', 'mail'],
  roleBaseDN: "OU=Groups,DC=domain,DC=com",
  roleFilter: "(&(objectClass=group)(member={{roleValue}}))",  //Filter groups where the UserDN is a member
  roleField: 'cn', //This is the role attribute from the groups LDAP entry.
  roleValue: 'dn' //This is the value from the User LDAP entry to perform a lookup on group membership
}

A search is done to find the user that exists and then perform another search to determine what groups the User is a member of by using the UserDN into the role/group search.

Thoughts?

@vesse
Copy link
Owner

vesse commented Apr 24, 2014

Hi,

This does not really seem like authentication related thing. From what I got from the explanation this would seem more like something you would implement in the verify callback.

@vesse vesse closed this as completed Jun 21, 2014
@vesse vesse reopened this Nov 5, 2014
@vesse
Copy link
Owner

vesse commented Nov 5, 2014

I've stumbled upon a similar need where memberof will not do. I'm thinking about adding option to fetch groups and add them to the user object so one could use those in verify callback. This needs changes also to node-ldapauth-fork.

@geekosaurusR3x
Copy link

In fact this will be a good idea :)

@CoinCoderBuffalo
Copy link

I added a 'group lookup' to ldap-auth-fork. Code is here: https://github.com/jjg77/node-ldapauth-fork/blob/ismemberof/lib/ldapauth.js.

The class takes 3 new opts: searchBaseGroups (required), searchFilterGroups (required), searchGroupAttributes (optional, but defaults to 'ismemberof')

Which can be configured in the passport LdapStrategy like this:

searchBaseGroups: 'ou=internal,o=company,c=us',
searchFilterGroups: '(&(uid={{username}})(objectclass=person)(ismemberof=*))',

The 'groups' get added to the user object and look like this:

[ 'cn=group1,ou=groups,o=company,c=us',
  'cn=group2,ou=groups,o=company,c=us',
  'cn=group3,ou=groups,o=company,c=us' ]

@vesse vesse closed this as completed in d4db47c Feb 24, 2015
@vesse
Copy link
Owner

vesse commented Feb 24, 2015

OK, this is now resolved with the update of ldapauth-fork. The new options are:

  • groupDnProperty: Optional, default 'dn'. The property of user object to use in {{dn}} interpolation of groupSearchFilter.
  • groupSearchBase: Optional. The base DN from which to search for groups. If defined, also groupSearchFilter must be defined for the search to work.
  • groupSearchScope: Optional, default sub.
  • groupSearchFilter: Optional. LDAP search filter for groups. The following literals are interpolated from the found user object: {{dn}} the property configured with groupDnProperty.
  • groupSearchAttributes: Optional, default all. Array of attributes to fetch from LDAP server.

eg.

var opts = {
  "server": {
    "url": "ldaps://ldap.example.com:636",
    "adminDn": "cn=LdapAdmin,dc=local",
    "adminPassword": "LdapAdminPassword",
    "searchBase": "dc=users,dc=local",
    "searchFilter": "(&(objectClass=person)(sAMAccountName={{username}}))",
    "searchAttributes": [
      "dn", "cn", "givenName", "name", "memberOf", "sAMAccountName"
    ],
    "groupSearchBase": "dc=groups,dc=local",
    "groupSearchFilter": "(member={{dn}})",
    "groupSearchAttributes": ["dn", "cn", "sAMAccountName"]
  }
};

@MayaLekova
Copy link

IMO this last answer is important enough to become part of the documentation. Also example with interpolation on property different than "dn" would be helpful.

@Djalmar
Copy link

Djalmar commented Mar 31, 2016

Please can you give an update to the documentation, i'm kinda new to LDAP

@UXabre
Copy link

UXabre commented Jun 13, 2016

Looking at the implementation, would it not make more sense to let the user create a dynamic group search query?

/lib/ldapauth.js:291

 var searchFilter = self.opts.groupSearchFilter.replace(/{{dn}}/g, user[self.opts.groupDnProperty]);

to be something like:

var searchFilter = self.opts.groupSearchFilter;

for( var property in user ) {
   searchFilter = searchFilter.replace(new RegExp('{{'+property+"}}', 'g'), user[property]);
}

This way, one could also, for instance, retrieve the CN for a primary group (currently my customer has a set-up like this, in which the CN for the group cannot be retrieved by memberof):

"groupSearchFilter": "(|(memberUid={{uid}})(gidNumber={{gidNumber}}))",

@vesse
Copy link
Owner

vesse commented Jun 13, 2016

@UXabre I don't really like the idea of looping over all properties of users when probably most of the replace calls would do nothing. I would however accept a pull request that does not break current functionality, but would enable giving a function(user) { return "groupSearchFilter"; } instead of just a string in groupSearchFilter. Then you could construct any filter but would not needlessly loop over the properties, and those who are happy using just one keyword (like myself) could do as they've done now.

@UXabre
Copy link

UXabre commented Jun 13, 2016

I agree, would be pointless to loop everything but the general idea is indeed what you propose. I'll see to make a pull request sometime this week :-)

@UXabre
Copy link

UXabre commented Jun 17, 2016

Meanwhile I have created the pull request for this extension and can be found here: vesse/node-ldapauth-fork#36

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

No branches or pull requests

7 participants