feathersjs-ecosystem / feathers-authentication-ldap

LDAP authentication strategy for feathers-authentication using Passport
MIT License
19 stars 4 forks source link

Compatible with feathers 4.3 #42

Open jondmoon opened 4 years ago

jondmoon commented 4 years ago

Since the release of @feathersjs/authentication^4.3.0, this module no longer works. I know there were a lot of changes with authentication in the 4.x branch, including moving away from using passport which this module uses. Are you planning to update this module to work with the new setup?

daffl commented 4 years ago

Probably not unless someone who is using this module has a chance to look into it.

Something that might also make more sense is to look at a custom authentication strategy that uses something like LDAPJS directly.

You can also still integrate the LDAP passport strategy like you would with any other Express application and then pass that information to the service call.

jondmoon commented 4 years ago

Okay, thanks. I was able to get something to work with ldapauth-fork (a wrapper over ldapjs). In case anyone is interested, here is the code that I used for authenticate-ldap.js:

const { AuthenticationBaseStrategy } = require('@feathersjs/authentication');
const LdapAuth = require('ldapauth-fork');

class LDAPStrategy extends AuthenticationBaseStrategy {
  verifyConfiguration() {
    const config = this.configuration;
    ['url', 'bindDN','bindCredentials', 'searchBase', 'searchFilter'].forEach(prop => {
      if (typeof config[prop] !== 'string') {
        throw new Error(`'${this.name}' authentication strategy requires a '${prop}' setting`);
      }
    });
  }
  authenticate(data) {
    return new Promise((resolve, reject) => {
      const username = data.username;
      const password = data.password;

      const auth = new LdapAuth(this.configuration);
      const name = this.name;
      const app = this.app;
      auth.authenticate(username, password, function (err, user) {
        if (err) {
          reject('Invalid username or password; logon denied');
        } else {
          resolve({
            authentication: { strategy: name },
            user
          });
        }
      });
    });
  }
}
exports.LDAPStrategy = LDAPStrategy;
daffl commented 4 years ago

This looks good. We could update this module to use that?

engineertdog commented 4 years ago

@jondmoon What settings did you use for LDAP? I wasn't able to authenticate with the settings I used.

var options = {
  url: "ldap://ldapserver1.companyName.com",
  bindDN: "uid=someUserAccount,ou=users,dc=companyName,dc=com",
  bindCredentials: 'someUserAccountPassword',
  searchBase: "dc=companyName,dc=com",
  searchFilter: "(uid={{sAMAccountName}})" // also tried uid={{username}}
};

Then for auth.authenticate, I used whatever the account name would be. For typical users, it follows the format last name + first initial as the username, which for another LDAP authentication library is shown as sAMAccountName

jondmoon commented 4 years ago

@engineertdog Here is what we are using (replacing actual names with pseudo-names): "ldap": { "name": "ldap", "bindDN": "DOMAIN\ldap-user", "bindCredentials": "****", "url": "ldaps://ldap.domain.com:636", "searchBase": "OU=Domain,DC=Domain,DC=com", "searchFilter": "(sAMAccountName={{username}})" } The search filter will be populated with the username field, so you have to include username, but it needs to match the field in your LDAP directory that actually has the username, which is sAMAccountName in Active Directory. I couldn't get the bindDN to work with the format that you are using, but was able to get it to work with this alternative syntax. I hope this helps.

engineertdog commented 4 years ago

@jondmoon Thanks for that, that was quite helpful as I'm not experienced with AD. However, when I tried to implement and test this, I had an error. I took the existing Feathers Chat (server + React) and looked to replace the login via local to ldap. Everything seems to be working as I get no errors from the Feathers client, but I'm unable to login. The login's catch block is triggered with an error of can not set subject from user._id. If I arbitrarily set user._id in the return object from that code above, then I get an error in React for no record found for id: <arbitrary id>

daffl commented 4 years ago

If you are going with a normal setup, the expectation is that the authentication strategy looks up or creates a new user on the users service and returns that:

const { AuthenticationBaseStrategy } = require('@feathersjs/authentication');
const LdapAuth = require('ldapauth-fork');

class LDAPStrategy extends AuthenticationBaseStrategy {
  verifyConfiguration() {
    const config = this.configuration;
    ['url', 'bindDN','bindCredentials', 'searchBase', 'searchFilter'].forEach(prop => {
      if (typeof config[prop] !== 'string') {
        throw new Error(`'${this.name}' authentication strategy requires a '${prop}' setting`);
      }
    });
  }
  authenticate(data) {
    return new Promise((resolve, reject) => {
      const username = data.username;
      const password = data.password;

      const auth = new LdapAuth(this.configuration);
      const name = this.name;
      const app = this.app;

      auth.authenticate(username, password, async (err, ldapUser) => {
        if (err) {
          reject('Invalid username or password; logon denied');
        } else {
          const userService = this.app.service('users');

          // Look up existing user
          const [ existingUser ] = await userService.find({
            paginate: false,
            query: {
              $limit: 1,
              ldapId: ldapUser.id // Customize to what you need here
            }
          });

          // Either use existing user or create new one
          const user = existingUser || await userService.create({
            // Create with data from ldapUser
            ldapId: ldapUser.id
          });

          resolve({
            authentication: { strategy: name },
            user
          });
        }
      });
    });
  }
}
exports.LDAPStrategy = LDAPStrategy;
daffl commented 4 years ago

This should basically be the next version of this module, just need to figure out a good way to test it in CI. I always found LDAP a little convoluted to set up.

engineertdog commented 4 years ago

@daffl I’m not sure on the possibilities of testing using CI; however, if you’d like some assistance in testing in general, I work in an AD environment.

daffl commented 4 years ago

Is there a way to spin up a super simple standalone LDAP server in a GitHub action?

engineertdog commented 4 years ago

I’m not too familiar with Github actions as I’ve only done CI with Jenkins and Gitlab, but as long as it follows the same principles, it should be. I found a docker container for running LDAP servers. May or may not need other containers associated with it.

Also, I’ll just make a note that the code above didn’t cause the authentication to fail if the user passed in the wrong password, or if there were any other errors. I had to modify the function to support that.

https://github.com/tiredofit/docker-openldap/blob/master/README.md

znerol commented 3 years ago

Is there a way to spin up a super simple standalone LDAP server in a GitHub action?

Maybe ldap-server-mock