Repository: moscajs/aedes
Pull request #419 (2020-02-09 05:55:15)

Allow user defined their error auth message
- User could define their own auth error message raised in `clientError`
- no custom error message / empty error message will be applied to the internal one
- User could define the CONNACK return code between 2 and 5. If out of ranges, 5 instead.


Patch:
diff --git a/lib/handlers/connect.js b/lib/handlers/connect.js
index d8586c4..7580c9c 100644
--- a/lib/handlers/connect.js
+++ b/lib/handlers/connect.js
@@ -121,21 +121,23 @@ function authenticate (arg, done) {
 
     if (err) {
       const errCode = err.returnCode
-      if (errCode && (errCode >= 1 && errCode <= 3)) {
+      if (errCode && (errCode >= 2 && errCode <= 5)) {
         arg.returnCode = errCode
       } else {
-        // If errorCode is 4 or not a number
-        arg.returnCode = 4
+        arg.returnCode = 5
+      }
+      if (!err.message) {
+        err.message = errorMessages[arg.returnCode]
       }
     } else {
       arg.returnCode = 5
+      err = new Error(errorMessages[arg.returnCode])
     }
-    var error = new Error(errorMessages[arg.returnCode])
-    error.errorCode = arg.returnCode
+    err.errorCode = arg.returnCode
     arg.client = client
     doConnack(arg,
       // [MQTT-3.2.2-5]
-      client.close.bind(client, done.bind(this, error)))
+      client.close.bind(client, done.bind(this, err)))
   }
 }
 


Test patch:
diff --git a/test/auth.js b/test/auth.js
index fc512e2..dbf6816 100644
--- a/test/auth.js
+++ b/test/auth.js
@@ -3,13 +3,13 @@
 const { test } = require('tap')
 const eos = require('end-of-stream')
 const Client = require('../lib/client')
-const { setup, connect, subscribe, subscribeMultiple } = require('./helper')
+const { setup, connect, noError, subscribe, subscribeMultiple } = require('./helper')
 const aedes = require('../')
 
 test('authenticate successfully a client with username and password', function (t) {
   t.plan(4)
 
-  const s = setup()
+  const s = noError(setup())
   t.tearDown(s.broker.close.bind(s.broker))
 
   s.broker.authenticate = function (client, username, password, cb) {
@@ -46,7 +46,7 @@ test('authenticate successfully a client with username and password', function (
 })
 
 test('authenticate unsuccessfully a client with username and password', function (t) {
-  t.plan(5)
+  t.plan(6)
 
   const s = setup()
   t.tearDown(s.broker.close.bind(s.broker))
@@ -58,6 +58,10 @@ test('authenticate unsuccessfully a client with username and password', function
     cb(null, false)
   }
 
+  s.broker.on('clientError', function (client, err) {
+    t.equal(err.errorCode, 5)
+  })
+
   s.broker.on('clientReady', function (client) {
     t.fail('client should not ready')
   })
@@ -73,7 +77,7 @@ test('authenticate unsuccessfully a client with username and password', function
       topic: null,
       payload: null,
       sessionPresent: false
-    }, 'unsuccessful connack')
+    }, 'unsuccessful connack, unauthorized')
   })
 
   eos(s.outStream, function () {
@@ -93,7 +97,7 @@ test('authenticate unsuccessfully a client with username and password', function
 })
 
 test('authenticate errors', function (t) {
-  t.plan(5)
+  t.plan(7)
 
   const s = setup()
   t.tearDown(s.broker.close.bind(s.broker))
@@ -105,6 +109,11 @@ test('authenticate errors', function (t) {
     cb(new Error('this should happen!'))
   }
 
+  s.broker.on('clientError', function (client, err) {
+    t.equal(err.message, 'this should happen!')
+    t.equal(err.errorCode, 5)
+  })
+
   s.broker.on('clientReady', function (client) {
     t.fail('client should not ready')
   })
@@ -112,7 +121,7 @@ test('authenticate errors', function (t) {
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
-      returnCode: 4,
+      returnCode: 5,
       length: 2,
       qos: 0,
       retain: false,
@@ -120,7 +129,7 @@ test('authenticate errors', function (t) {
       topic: null,
       payload: null,
       sessionPresent: false
-    }, 'unsuccessful connack')
+    }, 'unsuccessful connack, unauthorized')
   })
 
   eos(s.outStream, function () {
@@ -140,7 +149,7 @@ test('authenticate errors', function (t) {
 })
 
 test('authentication error when return code 1 (unacceptable protocol version) is passed', function (t) {
-  t.plan(5)
+  t.plan(7)
 
   const s = setup()
   t.tearDown(s.broker.close.bind(s.broker))
@@ -154,6 +163,11 @@ test('authentication error when return code 1 (unacceptable protocol version) is
     cb(error, null)
   }
 
+  s.broker.on('clientError', function (client, err) {
+    t.equal(err.message, 'Auth error')
+    t.equal(err.errorCode, 5)
+  })
+
   s.broker.on('clientReady', function (client) {
     t.fail('client should not ready')
   })
@@ -161,7 +175,7 @@ test('authentication error when return code 1 (unacceptable protocol version) is
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
-      returnCode: 1,
+      returnCode: 5,
       length: 2,
       qos: 0,
       retain: false,
@@ -169,7 +183,7 @@ test('authentication error when return code 1 (unacceptable protocol version) is
       topic: null,
       payload: null,
       sessionPresent: false
-    }, 'unsuccessful connack,unacceptable protocol version')
+    }, 'unsuccessful connack, unauthorized')
   })
 
   eos(s.outStream, function () {
@@ -189,7 +203,7 @@ test('authentication error when return code 1 (unacceptable protocol version) is
 })
 
 test('authentication error when return code 2 (identifier rejected) is passed', function (t) {
-  t.plan(5)
+  t.plan(7)
 
   const s = setup()
   t.tearDown(s.broker.close.bind(s.broker))
@@ -203,6 +217,11 @@ test('authentication error when return code 2 (identifier rejected) is passed',
     cb(error, null)
   }
 
+  s.broker.on('clientError', function (client, err) {
+    t.equal(err.message, 'Auth error')
+    t.equal(err.errorCode, 2)
+  })
+
   s.broker.on('clientReady', function (client) {
     t.fail('client should not ready')
   })
@@ -238,7 +257,7 @@ test('authentication error when return code 2 (identifier rejected) is passed',
 })
 
 test('authentication error when return code 3 (Server unavailable) is passed', function (t) {
-  t.plan(5)
+  t.plan(7)
 
   const s = setup()
   t.tearDown(s.broker.close.bind(s.broker))
@@ -252,6 +271,11 @@ test('authentication error when return code 3 (Server unavailable) is passed', f
     cb(error, null)
   }
 
+  s.broker.on('clientError', function (client, err) {
+    t.equal(err.message, 'Auth error')
+    t.equal(err.errorCode, 3)
+  })
+
   s.broker.on('clientReady', function (client) {
     t.fail('client should not ready')
   })
@@ -286,8 +310,62 @@ test('authentication error when return code 3 (Server unavailable) is passed', f
   })
 })
 
+test('authentication error when return code 4 (bad user or password) is passed', function (t) {
+  t.plan(7)
+
+  const s = setup()
+  t.tearDown(s.broker.close.bind(s.broker))
+
+  s.broker.authenticate = function (client, username, password, cb) {
+    t.ok(client instanceof Client, 'client is there')
+    t.equal(username, 'my username', 'username is there')
+    t.deepEqual(password, Buffer.from('my pass'), 'password is there')
+    var error = new Error('Auth error')
+    error.returnCode = 4
+    cb(error, null)
+  }
+
+  s.broker.on('clientError', function (client, err) {
+    t.equal(err.message, 'Auth error')
+    t.equal(err.errorCode, 4)
+  })
+
+  s.broker.on('clientReady', function (client) {
+    t.fail('client should not ready')
+  })
+
+  s.outStream.on('data', function (packet) {
+    t.deepEqual(packet, {
+      cmd: 'connack',
+      returnCode: 4,
+      length: 2,
+      qos: 0,
+      retain: false,
+      dup: false,
+      topic: null,
+      payload: null,
+      sessionPresent: false
+    }, 'unsuccessful connack, bad username or password')
+  })
+
+  eos(s.outStream, function () {
+    t.equal(s.broker.connectedClients, 0, 'no connected clients')
+  })
+
+  s.inStream.write({
+    cmd: 'connect',
+    protocolId: 'MQTT',
+    protocolVersion: 4,
+    clean: true,
+    clientId: 'my-client',
+    username: 'my username',
+    password: 'my pass',
+    keepalive: 0
+  })
+})
+
 test('authentication error when non numeric return code is passed', function (t) {
-  t.plan(5)
+  t.plan(7)
 
   const s = setup()
   t.tearDown(s.broker.close.bind(s.broker))
@@ -301,6 +379,11 @@ test('authentication error when non numeric return code is passed', function (t)
     cb(error, null)
   }
 
+  s.broker.on('clientError', function (client, err) {
+    t.equal(err.message, 'Non numeric error codes')
+    t.equal(err.errorCode, 5)
+  })
+
   s.broker.on('clientReady', function (client) {
     t.fail('client should not ready')
   })
@@ -308,7 +391,7 @@ test('authentication error when non numeric return code is passed', function (t)
   s.outStream.on('data', function (packet) {
     t.deepEqual(packet, {
       cmd: 'connack',
-      returnCode: 4,
+      returnCode: 5,
       length: 2,
       qos: 0,
       retain: false,
@@ -316,7 +399,7 @@ test('authentication error when non numeric return code is passed', function (t)
       topic: null,
       payload: null,
       sessionPresent: false
-    }, 'unsuccessful connack, bad user name or password')
+    }, 'unsuccessful connack, unauthorized')
   })
 
   eos(s.outStream, function () {
@@ -816,7 +899,7 @@ test('failed authentication does not disconnect other client with same clientId'
 })
 
 test('unauthorized connection should not unregister the correct one with same clientId', function (t) {
-  t.plan(2)
+  t.plan(4)
 
   const broker = aedes({
     authenticate: function (client, username, password, callback) {
@@ -832,6 +915,8 @@ test('unauthorized connection should not unregister the correct one with same cl
   t.tearDown(broker.close.bind(broker))
 
   broker.on('clientError', function (client, err) {
+    t.equal(err.message, 'bad user name or password')
+    t.equal(err.errorCode, 4)
     t.equal(broker.connectedClients, 1, 'my-client still connected')
   })