Improve performance, fix bug in authorization schema

examples/example.js

@@ -0,0 +1,28 @@
+'use strict'
+
+const fastify = require('fastify')({
+})
+const bearerAuthPlugin = require('..')
+const keys = new Set(['key'])
+
+fastify.register(bearerAuthPlugin, { keys })
+fastify.get('/foo', (req, reply) => {
+  reply.send({ authenticated: true })
+})
+
+fastify.listen({ port: 8000 }, (err) => {
+  if (err) {
+    fastify.log.error(err.message)
+    process.exit(1)
+  }
+  fastify.log.info('http://127.0.0.1:8000/foo')
+})
+
+// Missing Header
+// autocannon http://127.0.0.1:8000/foo
+// Invalid Bearer Type
+// autocannon -H authorization='Beaver key' http://127.0.0.1:8000/foo
+// Invalid Key
+// autocannon -H authorization='Bearer invalid' http://127.0.0.1:8000/foo
+// Valid Request
+// autocannon -H authorization='Bearer key' http://127.0.0.1:8000/foo

index.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const fp = require('fastify-plugin')
-const verifyBearerAuthFactory = require('./lib/verifyBearerAuthFactory')
+const verifyBearerAuthFactory = require('./lib/verify-bearer-auth-factory')
 
 function fastifyBearerAuth (fastify, options, done) {
   const defaultLogLevel = 'error'

lib/verifyBearerAuthFactory.js → lib/verify-bearer-auth-factory.js

@@ -24,31 +24,33 @@ module.exports = function verifyBearerAuthFactory (options) {
     keys[i] = Buffer.from(keys[i])
   }
 
-  return function verifyBearerAuth (request, reply, done) {
-    function authorizationHeaderErrorFn (errorMessage) {
-      const noHeaderError = Error(errorMessage)
-      if (verifyErrorLogLevel) request.log[verifyErrorLogLevel]('unauthorized: %s', noHeaderError.message)
-      if (contentType) reply.header('content-type', contentType)
-      reply.code(401)
-      if (!addHook) {
-        done(noHeaderError)
-        return
-      }
-      reply.send(errorResponse(noHeaderError))
+  const bearerTypePrefix = bearerType + ' '
+  const bearerTypePrefixLength = bearerType.length + 1
+
+  function handleUnauthorized (request, reply, done, message) {
+    const noHeaderError = Error(message)
+    if (verifyErrorLogLevel) request.log[verifyErrorLogLevel]('unauthorized: %s', noHeaderError.message)
+    if (contentType) reply.header('content-type', contentType)
+    reply.code(401)
+    if (!addHook) {
+      done(noHeaderError)
+      return
     }
+    reply.send(errorResponse(noHeaderError))
+  }
 
-    const header = request.raw.headers.authorization
-    if (!header) {
-      return authorizationHeaderErrorFn('missing authorization header')
+  return function verifyBearerAuth (request, reply, done) {
+    const authorizationHeader = request.raw.headers.authorization
+    if (!authorizationHeader) {
+      return handleUnauthorized(request, reply, done, 'missing authorization header')
     }
 
-    const type = header.substring(0, bearerType.length)
-    if (type !== bearerType) {
-      return authorizationHeaderErrorFn('invalid authorization header')
+    if (authorizationHeader.substring(0, bearerTypePrefixLength) !== bearerTypePrefix) {
+      return handleUnauthorized(request, reply, done, 'invalid authorization header')
     }
 
-    const key = header.substring(bearerType.length).trim()
-    let retVal
+    const key = authorizationHeader.substring(bearerTypePrefixLength).trim()
+    let retVal = false
     // check if auth function is defined
     if (auth && auth instanceof Function) {
       try {
@@ -62,22 +64,16 @@ module.exports = function verifyBearerAuthFactory (options) {
       retVal = authenticate(keys, key)
     }
 
-    const invalidKeyError = Error('invalid authorization header')
-
     // retVal contains the result of the auth function if defined or the
     // result of the key comparison.
     // retVal is enclosed in a Promise.resolve to allow auth to be a normal
-    // function or an async funtion. If it returns a non-promise value it
+    // function or an async function. If it returns a non-promise value it
     // will be converted to a resolving promise. If it returns a promise it
     // will be resolved.
     Promise.resolve(retVal).then((val) => {
       // if val is not truthy return 401
       if (val === false) {
-        if (verifyErrorLogLevel) request.log[verifyErrorLogLevel]('unauthorized: %s', invalidKeyError.message)
-        if (contentType) reply.header('content-type', contentType)
-        reply.code(401)
-        if (!addHook) return done(invalidKeyError)
-        reply.send(errorResponse(invalidKeyError))
+        handleUnauthorized(request, reply, done, 'invalid authorization header')
         return
       }
       if (val === true) {

test/integration.test.js

@@ -43,6 +43,22 @@ test('invalid key route fails correctly', (t) => {
   })
 })
 
+test('missing space between bearerType and key fails correctly', (t) => {
+  t.plan(2)
+  fastify.inject({
+    method: 'GET',
+    url: '/test',
+    headers: {
+      authorization: 'Bearer123456'
+    }
+  }).then(response => {
+    t.equal(response.statusCode, 401)
+    t.match(JSON.parse(response.body).error, /invalid authorization header/)
+  }).catch(err => {
+    t.error(err)
+  })
+})
+
 test('missing header route fails correctly', (t) => {
   t.plan(2)
   fastify.inject({ method: 'GET', url: '/test' }).then(response => {
@@ -58,7 +74,7 @@ test('integration with @fastify/auth', async (t) => {
 
   const fastify = require('fastify')()
   await fastify.register(plugin, { addHook: false, keys: new Set(['123456']) })
-  await fastify.decorate('allowAnonymous', function (request, _, done) {
+  fastify.decorate('allowAnonymous', function (request, _, done) {
     if (!request.headers.authorization) {
       return done()
     }
@@ -119,7 +135,7 @@ test('integration with @fastify/auth; not the last auth option', async (t) => {
 
   const fastify = require('fastify')()
   await fastify.register(plugin, { addHook: false, keys: new Set(['123456']) })
-  await fastify.decorate('alwaysValidAuth', function (request, _, done) {
+  fastify.decorate('alwaysValidAuth', function (request, _, done) {
     return done()
   })
   await fastify.register(require('@fastify/auth'))

test/verifyBearerAuthFactory.test.js → test/verify-bearer-auth-factory.test.js

@@ -2,7 +2,7 @@
 
 const test = require('tap').test
 const noop = () => {}
-const verifyBearerAuthFactory = require('../lib/verifyBearerAuthFactory')
+const verifyBearerAuthFactory = require('../lib/verify-bearer-auth-factory')
 const key = '123456789012354579814'
 const keys = { keys: new Set([key]) }
 
@@ -218,6 +218,29 @@ test('hook accepts correct header and alternate Bearer', (t) => {
   })
 })
 
+test('hook throws if header misses at least one space after bearerType', (t) => {
+  t.plan(2)
+
+  const request = {
+    log: { error: noop },
+    raw: {
+      headers: { authorization: `Bearer${key}` }
+    }
+  }
+  const response = {
+    code: () => response,
+    send
+  }
+
+  function send (body) {
+    t.ok(body.error)
+    t.match(body.error, /invalid authorization header/)
+  }
+
+  const hook = verifyBearerAuthFactory(keys)
+  hook(request, response)
+})
+
 test('hook accepts correct header with extra padding', (t) => {
   t.plan(1)