Support for LDAP

[Jose96GIT]

Hi! I've implemented a LDAP support for this module.

The major doubt that I have with this implementation is about the modules names nomenclature, because it could be a little confusing, for example with implementFindOrCreateUser function.

I also want to comment a problem that I've found doing this implementation, and it's that I couldn't concat LDAP strategy with local strategy in the same button action, I mean, when LDAP's authentication failed, I couldn't recover the username and password introduced by the user before and try again with a local login. Is there any way with Apostrophe to recover this data or the last request?

[boutell]

Hi Jose, what does a complete configuration to use this feature look like? Are you still configuring it to use passport-ldap or a similar module, and you need some extra "glue" in the module? Where does the existing implementation fail to be compatible — maybe we can do something more general that will also help other passport modules? Thanks.

[Jose96GIT]

Hi Tom, maybe I wasn't explaining as well as I should, so I'm going to explain all step by step, so that it will be easier to understand.

1. I'm using passport-ldapauth, and the first we should do is declare the strategy with it, as in this simple example.

strategies: [
    {
       module: 'passport-ldapauth',
        options: {
          server: {
            url: 'ldap://localhost:10389',
            bindDN: 'uid=admin,ou=system',
            bindCredentials: 'secret',
            searchBase: 'ou=people,dc=example,dc=com',
            searchFilter: '(uid={{username}})',
         }
      }
   },
]

2. This strategy behaviour is different to other like Gitlab or Google, so I needed to use a POST route an also change the beaviour of the strategy's callback.

Change in enablePassportStrategies function

if (spec.name !== 'ldapauth')
   self.strategies[spec.name] = new Strategy(spec.options, self.findOrCreateUser(spec));
else 
   self.strategies[spec.name] = new Strategy(spec.options, self.ldapFindOrCreateUser(spec));

Change in addLoginRoute function

if (spec.name !== 'ldapauth')
   self.apos.app.get(self.getLoginUrl(spec), self.apos.login.passport.authenticate(spec.name, spec.authenticate));
else 
   self.apos.app.post(self.getLoginUrl(spec), self.apos.login.passport.authenticate(spec.name, {successRedirect: '/',failureRedirect: '/login?error=1'}));

3. I've created a function so that we can addapt the LDAP profile to Apostrophe user structure easily, also as I changed the LDAP callback, I've created an other function to implement it.

New callback function to addapt LDAP profile and also pass parameters in a correct order

self.ldapFindOrCreateUser = function(spec) {
  return function(profile, callback) {
    self.completeLdapProfile(profile);
    self.implementFindOrCreateUser(spec, "", "", profile, callback);
  }
 };

I also moved the implementation of findOrCreateUser function to another function so that there's no duplicated code between custom callback and ldapCallback.

Finally, the problem is that if LDAP authentication fails, it send me to the failureRoute that I set before in the express route definition, but without the request and the response. So I couldn't redirect with the same user data to another strategy. Is it possible to recover the request or maybe to save this information temporarily?

I hope, this answers all your questions.

[boutell]

Thanks, this is very helpful.

It sounds like we might be able to create some flags and other configuration options for the individual changes in behavior, and then have some presets that get activated based on the strategy name if it's one like LDAP that we know about. That's the direction I would like to see this take, rather than hardcoding a lot of if statements specifically for LDAP.

[boutell]

That way, if someone wants to use a strategy we're unfamiliar with but the tweaks they need are similar to those needed for LDAP, they can set those flags.

[Jose96GIT]

I've changed it. I used two different tags, the first to change the GET route to a POST route, and the second to change the callback method.

[boutell]

This is a big step in the right direction. I'm thinking that the module should contain an object like this with preconfigured tweaks for modules that need them:

self.strategyOverrides = {
  'passport-ldap': {
    postRoute: true,
    mapFields: {
      username: 'uid'
    }
  }
};

We would then merge these options in automatically so the developer doesn't have to tell us what we already know about passport-ldap, but we don't have to put a ton of if passport-ldap... statements all around the module either.

I would think we can make a single findOrCreateUser method work for all cases if we do a good job with this.