Merge pull request #60 from lneveu/master

Fix authentication issues (fix #47)

README.md

@@ -21,6 +21,12 @@ auth:
     type: ldap
     # Only required if you are fetching groups that do not have a "cn" property. defaults to "cn"
     groupNameAttribute: "ou"
+    # Optional, default false.
+    cache:
+      # max credentials to cache (default to 100 if cache is enabled)
+      size: 100
+      # cache expiration in seconds (default to 300 if cache is enabled)
+      expire: 300
     client_options:
       url: "ldaps://ldap.example.com"
       # Only required if you need auth to bind
@@ -36,10 +42,6 @@ auth:
       searchAttributes: ['*', 'memberOf']
       # Else, if you don't (use one or the other):
       # groupSearchFilter: '(memberUid={{dn}})'
-      #
-      # Optional, default false.
-      # If true, then up to 100 credentials at a time will be cached for 5 minutes.
-      cache: false
       # Optional
       reconnect: true
 ```

index.js

@@ -1,9 +1,20 @@
 const Promise = require('bluebird');
 const rfc2253 = require('rfc2253');
 const LdapAuth = require('ldapauth-fork');
+const Cache = require('ldapauth-fork/lib/cache');
+const bcrypt = require('bcryptjs');
 
 Promise.promisifyAll(LdapAuth.prototype);
 
+function authenticatedUserGroups(user, groupNameAttribute) {
+  return [
+    user.cn,
+    // _groups or memberOf could be single els or arrays.
+    ...user._groups ? [].concat(user._groups).map((group) => group[groupNameAttribute]) : [],
+    ...user.memberOf ? [].concat(user.memberOf).map((groupDn) => rfc2253.parse(groupDn).get('CN')) : [],
+  ];
+}
+
 function Auth(config, stuff) {
   const self = Object.create(Auth.prototype);
   self._users = {};
@@ -17,17 +28,18 @@ function Auth(config, stuff) {
   // pass verdaccio logger to ldapauth
   self._config.client_options.log = stuff.logger;
 
+  // always set ldapauth cache false
+  self._config.client_options.cache = false;
+
   // TODO: Set more defaults
   self._config.groupNameAttribute = self._config.groupNameAttribute || 'cn';
 
-  // ldap client
-  self._ldapClient = new LdapAuth(self._config.client_options);
-
-  self._ldapClient.on('error', (err) => {
-    self._logger.warn({
-      err: err,
-    }, `LDAP error ${err}`);
-  });
+  if (config.cache) {
+    const size = typeof config.cache.size === 'number' ? config.cache.size : 100;
+    const expire = typeof config.cache.expire === 'number' ? config.cache.expire : 300;
+    self._userCache = new Cache(size, expire, stuff.logger, 'user');
+    self._salt = bcrypt.genSaltSync();
+  }
 
   return self;
 }
@@ -37,27 +49,52 @@ module.exports = Auth;
 //
 // Attempt to authenticate user against LDAP backend
 //
-Auth.prototype.authenticate = function (user, password, callback) {
-
-  this._ldapClient.authenticateAsync(user, password)
-    .then((ldapUser) => {
-      if (!ldapUser) return [];
-
-      return [
-        ldapUser.cn,
-        // _groups or memberOf could be single els or arrays.
-        ...ldapUser._groups ? [].concat(ldapUser._groups).map((group) => group[this._config.groupNameAttribute]) : [],
-        ...ldapUser.memberOf ? [].concat(ldapUser.memberOf).map((groupDn) => rfc2253.parse(groupDn).get('CN')) : [],
-      ];
+Auth.prototype.authenticate = function (username, password, callback) {
+
+  if (this._config.cache) {
+    const cached = this._userCache.get(username);
+    if (cached && bcrypt.compareSync(password, cached.password)) {
+      const userGroups = authenticatedUserGroups(cached.user, this._config.groupNameAttribute);
+      userGroups.cacheHit = true;
+      return callback(null, userGroups);
+    }
+  }
+
+  // ldap client
+  const ldapClient = new LdapAuth(this._config.client_options);
+
+  ldapClient.authenticateAsync(username, password)
+    .then((user) => {
+      if (!user) {
+        return [];
+      }
+
+      if (this._config.cache) {
+        try {
+          const hash = bcrypt.hashSync(password, this._salt);
+          this._userCache.set(username, { password: hash, user });
+        } catch(err) {
+          this._logger.warn({ username, err }, `verdaccio-ldap bcrypt hash error ${err}`);
+        }
+      }
+
+      return authenticatedUserGroups(user, this._config.groupNameAttribute);
     })
     .catch((err) => {
       // 'No such user' is reported via error
-      this._logger.warn({
-        user: user,
-        err: err,
-      }, `LDAP error ${err}`);
-
+      this._logger.warn({ username, err }, `verdaccio-ldap error ${err}`);
       return false; // indicates failure
     })
+    .finally(() => {
+      // This will do nothing with Node 10 (https://github.com/joyent/node-ldapjs/issues/483)
+      ldapClient.closeAsync()
+        .catch((err) => {
+          this._logger.warn({ err }, `verdaccio-ldap error on close ${err}`);
+        });
+    })
     .asCallback(callback);
+
+  ldapClient.on('error', (err) => {
+    this._logger.warn({ err }, `verdaccio-ldap error ${err}`);
+  });
 };

package.json

@@ -8,6 +8,7 @@
     "url": "git://github.com/Alexandre-io/verdaccio-ldap.git"
   },
   "dependencies": {
+    "bcryptjs": "^2.4.3",
     "bluebird": "~3.5.2",
     "ldapauth-fork": "~4.2.0",
     "rfc2253": "~0.1.1"
@@ -20,7 +21,7 @@
     "mocha": "~6.1.0"
   },
   "scripts": {
-    "pretest": "./node_modules/jshint/bin/jshint .",
+    "pretest": "jshint .",
     "test": "mocha --exit tests/bootstrap.spec.js tests/**/*.spec.js"
   },
   "keywords": [

tests/integration/test.spec.js

@@ -28,4 +28,34 @@ describe('ldap auth', function () {
     });
 
   });
+
+  it('should use cache', function (done) {
+    const auth = new Auth({
+      cache: true,
+      client_options: {
+        url: "ldap://localhost:4389",
+        searchBase: 'ou=users,dc=myorg,dc=com',
+        searchFilter: '(&(objectClass=posixAccount)(!(shadowExpire=0))(uid={{username}}))',
+        groupDnProperty: 'cn',
+        groupSearchBase: 'ou=groups,dc=myorg,dc=com',
+        // If you have memberOf:
+        searchAttributes: ['*', 'memberOf'],
+        // Else, if you don't:
+        // groupSearchFilter: '(memberUid={{dn}})',
+      }
+    }, { logger: log });
+
+    auth.authenticate('user', 'password', function (err, results) {
+      (err === null).should.be.true;
+      should.not.exist(results.cacheHit);
+      results[0].should.equal('user');
+
+      auth.authenticate('user', 'password', function (err, results) {
+        (err === null).should.be.true;
+        results.cacheHit.should.be.true;
+        results[0].should.equal('user');
+        done();
+      });
+    });
+  });
 });