Add error header for when clients must be upgraded

dadi/lib/auth/index.js

@@ -72,6 +72,26 @@ AuthMiddleware.prototype.handleInvalidCredentials = function (req, res, next) {
   next(error)
 }
 
+/**
+ * Handles the case where the client record needs to be upgraded.
+ *
+ * @param  {http.IncomingMessage}   req
+ * @param  {http.ServerResponse}    res
+ * @param  {Function}               next
+ */
+AuthMiddleware.prototype.handleClientInNeedOfUpgrade = function (req, res, next) {
+  let error = new Error('Client must be upgraded')
+
+  error.statusCode = 401
+
+  res.setHeader(
+    'WWW-Authenticate',
+    'Bearer, error="client_needs_upgrade", error_description="The client record on the server must be upgraded"'
+  )
+
+  next(error)
+}
+
 /**
  * Takes the client ID and secret present in the request body and
  * attempts to generate a bearer token with them. If successful,
@@ -127,6 +147,10 @@ AuthMiddleware.prototype.generateToken = function (req, res, next) {
       return res.end(JSON.stringify(response))
     })
   }).catch(error => {
+    if (error.message === 'CLIENT_NEEDS_UPGRADE') {
+      return this.handleClientInNeedOfUpgrade(req, res, next)
+    }
+
     return help.sendBackJSON(null, res, next)(error)
   })
 }

dadi/lib/model/acl/client.js

@@ -3,6 +3,10 @@ const bcrypt = require('bcrypt')
 const config = require('./../../../../config.js')
 const roleModel = require('./role')
 
+// This value should be incremented if we ever replace bcrypt with another
+// hashing algorithm.
+const HASH_VERSION = 1
+
 const Client = function () {
   this.schema = {
     clientId: {
@@ -87,6 +91,7 @@ Client.prototype.create = function (client, {
 
     return this.hashSecret(client.secret)
   }).then(hashedsecret => {
+    client._hashVersion = HASH_VERSION
     client.secret = hashedsecret
 
     return this.model.create({
@@ -167,8 +172,9 @@ Client.prototype.get = function (clientId, secret) {
     const {results} = response
     const mustValidateSecret = results.length === 1 &&
       typeof secret === 'string'
+    const [record] = results
     const secretValidation = mustValidateSecret
-      ? this.validateSecret(results[0].secret, secret)
+      ? this.validateSecret(record.secret, secret, record._hashVersion)
       : Promise.resolve(true)
 
     return secretValidation.then(secretIsValid => {
@@ -513,6 +519,11 @@ Client.prototype.roleRemove = function (clientId, roles) {
   })
 }
 
+/**
+ * Sets an internal reference to the instance of Model.
+ *
+ * @param {Object} model
+ */
 Client.prototype.setModel = function (model) {
   this.model = model
 }
@@ -552,6 +563,7 @@ Client.prototype.update = function (clientId, update) {
     return this.model.find({
       options: {
         fields: {
+          _hashVersion: 1,
           data: 1,
           secret: 1
         }
@@ -565,12 +577,15 @@ Client.prototype.update = function (clientId, update) {
       )
     }
 
+    const [record] = results
+
     // If a `currentSecret` property was sent, we must validate it against the
     // hashed secret in the database.
     if (typeof currentSecret === 'string') {
       return this.validateSecret(
-        results[0].secret,
-        currentSecret
+        record.secret,
+        currentSecret,
+        record._hashVersion
       ).then(secretIsValid => {
         if (!secretIsValid) {
           return Promise.reject(
@@ -588,6 +603,7 @@ Client.prototype.update = function (clientId, update) {
     // before sending it to the database.
     if (typeof secret === 'string') {
       return this.hashSecret(secret).then(hashedSecret => {
+        update._hashVersion = HASH_VERSION
         update.secret = hashedSecret
 
         return results
@@ -688,7 +704,13 @@ Client.prototype.validate = function (client, {
  * @param  {String}   candidate
  * @return {Promise<Boolean>}
  */
-Client.prototype.validateSecret = function (hash, candidate) {
+Client.prototype.validateSecret = function (hash, candidate, hashVersion) {
+  if (hashVersion !== HASH_VERSION) {
+    return Promise.reject(
+      new Error('CLIENT_NEEDS_UPGRADE')
+    )
+  }
+
   return bcrypt.compare(candidate, hash)
 }
 

test/acceptance/acl/token-store.js

@@ -50,7 +50,39 @@ describe('Token store', () => {
         .expect('content-type', 'application/json')
         .expect('pragma', 'no-cache')
         .expect('Cache-Control', 'no-store')
-        .expect(401, done)
+        .expect(401, (err, res) => {
+          res.headers['www-authenticate'].includes('error="invalid_credentials"').should.eql(true)
+          res.headers['www-authenticate'].includes('error_description="Invalid credentials supplied"').should.eql(true)
+
+          done(err)
+        })
+    })
+
+    it('should return 401 with a specific error message in the www-authenticate header if the client must be upgraded due to an outdated hashing algorithm', done => {
+      const unhashedClient = {
+        clientId: 'anotherTestClient',
+        secret: 'iShouldBeHashed',
+        accessType: 'admin',
+        _hashVersion: null
+      }
+
+      help.createClient(unhashedClient, () => {
+        client
+        .post(tokenRoute)
+        .send({
+          clientId: unhashedClient.clientId,
+          secret: unhashedClient.secret
+        })
+        .expect('content-type', 'application/json')
+        .expect('pragma', 'no-cache')
+        .expect('Cache-Control', 'no-store')
+        .expect(401, (err, res) => {
+          res.headers['www-authenticate'].includes('error="client_needs_upgrade"').should.eql(true)
+          res.headers['www-authenticate'].includes('error_description="The client record on the server must be upgraded"').should.eql(true)
+
+          done(err)
+        })
+      })
     })
 
     it('should return 401 if the client ID or secret contain non-string values', done => {

test/acceptance/help.js

@@ -7,6 +7,22 @@ const connection = require(__dirname + '/../../dadi/lib/model/connection')
 const config = require(__dirname + '/../../config')
 const request = require('supertest')
 
+function hashClientSecret(client) {
+  switch (client._hashVersion) {
+    case undefined:
+    case 1:
+      return Object.assign({}, client, {
+        _hashVersion: 1,
+        secret: bcrypt.hashSync(client.secret, config.get('auth.saltRounds'))
+      })
+
+    case null:
+      delete client._hashVersion
+
+      return client
+  }
+}
+
 module.exports.bulkRequest = function ({method = 'get', requests, token}) {
   const client = request(`http://${config.get('server.host')}:${config.get('server.port')}`)
   let results = []
@@ -130,7 +146,9 @@ module.exports.dropDatabase = function (database, collectionName, done) {
   }
 }
 
-module.exports.createClient = function (client, done) {
+module.exports.createClient = function (client, done, {
+  hashVersion = 1
+} = {}) {
   if (!client) {
     client = {
       accessType: 'admin',
@@ -139,9 +157,7 @@ module.exports.createClient = function (client, done) {
     }
   }
 
-  client = Object.assign({}, client, {
-    secret: bcrypt.hashSync(client.secret, config.get('auth.saltRounds'))
-  })
+  client = hashClientSecret(client)
 
   var collectionName = config.get('auth.clientCollection')
   var conn = connection({
@@ -191,9 +207,7 @@ module.exports.createACLClient = function (client, callback) {
     config.get('datastore')
   )
 
-  client = Object.assign({}, client, {
-    secret: bcrypt.hashSync(client.secret, config.get('auth.saltRounds'))
-  })
+  client = hashClientSecret(client)
 
   return clientsConnection.datastore.insert({
     data: client,