@ministryofjustice/fb-jwt-client-node - npm Package Compare versions

Comparing version 0.0.29 to 0.0.30

lib/fb-jwt-client.js

 const got = require('got')
-const merge = require('got/source/merge')
 const jwt = require('jsonwebtoken')
@@ -16,11 +15,24 @@ const pathToRegexp = require('path-to-regexp')
 const getResponseLabels = (response) => {
-  const responseLabels = {
-    status_code: response.statusCode
+  const responseLabels = {}
+
+  if (response.status) {
+    responseLabels.status = response.status
   }
+
+  if (response.statusCode) {
+    responseLabels.status_code = response.statusCode
+  }
+
   if (response.statusMessage) {
     responseLabels.status_message = response.statusMessage
   }
+
   if (response.name) {
     responseLabels.error_name = response.name
   }
+
+  if (response.message) {
+    responseLabels.error_message = response.message
+  }
+
   return responseLabels
@@ -89,9 +101,10 @@ }
     // provide default Prometheus startTimer behaviour so as not to have to wrap all instrumentation calls in conditionals
-    const defaultMetrics = {
-      startTimer: () => {
-        return () => {}
-      }
+    const startTimer = () => () => ({})
+
+    this.apiMetrics = {
+      startTimer
     }
-    this.apiMetrics = Object.assign({}, defaultMetrics)
-    this.requestMetrics = Object.assign({}, defaultMetrics)
+    this.requestMetrics = {
+      startTimer
+    }
   }
@@ -127,7 +140,5 @@
   generateAccessToken (data) {
-    // NB. jsonwebtoken helpfully sets ‘iat’ option by default
     const checksum = crypto.createHash('sha256').update(JSON.stringify(data)).digest('hex')
-    const payload = {checksum}
-    const accessToken = jwt.sign(payload, this.serviceToken, {algorithm})
-    return accessToken
+
+    return jwt.sign({checksum}, this.serviceToken, {algorithm})
   }
@@ -152,5 +163,3 @@
   encrypt (token, data, ivSeed) {
-    const dataString = JSON.stringify(data)
-    const encryptedData = aes256.encrypt(token, dataString, ivSeed)
-    return encryptedData
+    return aes256.encrypt(token, JSON.stringify(data), ivSeed)
   }
@@ -164,3 +173,3 @@
    *
-   * @param {string} encryptedData
+   * @param {string} data
    * Encrypted data
@@ -172,11 +181,8 @@ *
    **/
-  decrypt (token, encryptedData) {
-    let data
+  decrypt (token, data) {
     try {
-      data = aes256.decrypt(token, encryptedData)
-      data = JSON.parse(data)
+      return JSON.parse(aes256.decrypt(token, data))
     } catch (e) {
       this.throwRequestError(500, 'EINVALIDPAYLOAD')
     }
-    return data
   }
@@ -195,9 +201,10 @@
    *
+   * @param {string} [ivSeed]
+   * Initialization Vector
+   *
    * @return {string}
    *
    **/
-  encryptUserIdAndToken (userId, userToken) {
-    const serviceSecret = this.serviceSecret
-    const ivSeed = userId + userToken
-    return this.encrypt(serviceSecret, {userId, userToken}, ivSeed)
+  encryptUserIdAndToken (userId, userToken, ivSeed = userId + userToken) {
+    return this.encrypt(this.serviceSecret, {userId, userToken}, ivSeed)
   }
@@ -215,4 +222,3 @@
   decryptUserIdAndToken (encryptedData) {
-    const serviceSecret = this.serviceSecret
-    return this.decrypt(serviceSecret, encryptedData)
+    return this.decrypt(this.serviceSecret, encryptedData)
   }
@@ -235,4 +241,4 @@
     const toPath = pathToRegexp.compile(urlPattern)
-    const endpointUrl = this.serviceUrl + toPath(context)
-    return endpointUrl
+
+    return this.serviceUrl + toPath(context)
   }
@@ -252,3 +258,3 @@
    *
-   * @param {boolean} [searchParams]
+   * @param {boolean} [isGET]
    * Send payload as query string
@@ -260,7 +266,7 @@ *
    **/
-  createRequestOptions (urlPattern, context, data = {}, searchParams) {
+  createRequestOptions (urlPattern, context, data = {}, isGET) {
     const accessToken = this.generateAccessToken(data)
     const url = this.createEndpointUrl(urlPattern, context)
-    const hasData = Object.keys(data).length
-    const json = hasData && !searchParams ? data : true
+    const hasData = !!Object.keys(data).length
+
     const requestOptions = {
@@ -270,13 +276,16 @@ url,
         'x-access-token': accessToken
-      }
+      },
+      responseType: 'json'
     }
-    if (hasData && !searchParams) {
-      requestOptions.body = json
-    }
-    if (searchParams && hasData) {
-      requestOptions.searchParams = {
-        payload: Buffer.from(JSON.stringify(data)).toString('Base64')
+
+    if (hasData) {
+      if (isGET) {
+        requestOptions.searchParams = {
+          payload: Buffer.from(JSON.stringify(data)).toString('Base64')
+        }
+      } else {
+        requestOptions.body = data
       }
     }
-    requestOptions.json = true
+
     return requestOptions
@@ -286,11 +295,39 @@ }
   logError (type, error, labels, logger) {
-    const errorResponse = error.error || error.body
-    const errorResponseObj = typeof errorResponse === 'object' ? JSON.stringify(errorResponse) : ''
+    if (Reflect.has(error, 'gotOptions')) {
+      const {
+        gotOptions: {
+          headers
+        }
+      } = error
 
-    if (error.gotOptions) {
-      error.client_headers = error.gotOptions.headers
+      error.client_headers = headers
     }
+
+    const {
+      client_name: name
+    } = labels
+
+    if ((error.body || false) instanceof Object) error.error = error.body
+
     const logObject = Object.assign({}, labels, {error})
 
-    logger.error(logObject, `JWT ${type} request error: ${this.constructor.name}: ${labels.method.toUpperCase()} ${labels.base_url}${labels.url} - ${error.name} - ${error.code ? error.code : ''} - ${error.statusCode ? error.statusCode : ''} - ${error.statusMessage ? error.statusMessage : ''} - ${errorResponseObj}`)
+    /*
+     *  This is ugly but at least it's not a single super long line of stupid
+     */
+    const logMessage = `JWT ${type} request error: ${name}:`
+      .concat(' ')
+      .concat(labels.method.toUpperCase())
+      .concat(' ')
+      .concat([
+        labels.base_url.concat(labels.url),
+        error.name || '',
+        error.code || '',
+        error.statusCode || '',
+        error.statusMessage || ''
+      ].join(' - ').concat(' - '))
+      .concat(
+        error.error ? JSON.stringify(error.error) : ''
+      )
+
+    logger.error(logObject, logMessage)
   }
@@ -331,8 +368,9 @@
       payload,
-      sendOptions
+      sendOptions = {}
     } = args
+
     const client = this
     const client_name = this.constructor.name // eslint-disable-line camelcase
     const base_url = this.serviceUrl // eslint-disable-line camelcase
-    const options = this.createRequestOptions(url, context, payload, method === 'get')
+    const requestOptions = this.createRequestOptions(url, context, payload, method === 'get')
 
@@ -346,17 +384,13 @@ const labels = {
 
-    const logError = (type, e) => {
-      const errorType = `jwt_${type.toLowerCase()}_request_error`
-      const logLabels = Object.assign({}, labels, {
-        name: errorType
-      })
-      client.logError(type, e, logLabels, logger)
-    }
-
     let requestMetricsEnd
     let retryCounter = 1
 
-    const gotOptions = merge.options(got.defaults.options, {
+    function logError (type, error) {
+      client.logError(type, error, Object.assign({}, labels, {name: `jwt_${type.toLowerCase()}_request_error`}), logger)
+    }
+
+    const gotOptions = got.mergeOptions(got.defaults.options, {
       hooks: {
         beforeRequest: [
-          (options, error, retryCount) => {
+          () => {
             requestMetricsEnd = this.requestMetrics.startTimer(labels)
@@ -367,8 +401,7 @@ }
           (options, error, retryCount) => {
-            error.retryCount = retryCount
-            retryCounter = retryCount
+            error.retryCount = retryCounter = retryCount
+
             logError('client', error)
-            if (requestMetricsEnd) {
-              requestMetricsEnd(getResponseLabels(error))
-            }
+
+            if (requestMetricsEnd) requestMetricsEnd(getResponseLabels(error))
             requestMetricsEnd = this.requestMetrics.startTimer(labels)
@@ -380,3 +413,3 @@ }
             error.retryCount = retryCounter
-            requestMetricsEnd(getResponseLabels(error))
+            if (requestMetricsEnd) requestMetricsEnd(getResponseLabels(error))
             return error
@@ -386,5 +419,11 @@ }
         afterResponse: [
-          (response, retryWithMergedOptions) => {
+          (response) => {
             if (response.statusCode >= 400) {
-              const {statusCode, statusMessage, body, retryCount} = response
+              const {
+                statusCode,
+                statusMessage,
+                body,
+                retryCount
+              } = response
+
               const error = {
@@ -396,6 +435,8 @@ statusCode,
               }
+
               logError('client', error)
             }
-            requestMetricsEnd(getResponseLabels(response))
-            response.body = response.body || '{}'
+
+            if (requestMetricsEnd) requestMetricsEnd(getResponseLabels(response))
+            response.body = response.body || {}
             return response
@@ -405,3 +446,3 @@ }
       }
-    }, sendOptions, options)
+    }, sendOptions, requestOptions)
 
@@ -412,18 +453,23 @@ const apiMetricsEnd = this.apiMetrics.startTimer(labels)
       const response = await got[method](gotOptions)
+
       apiMetricsEnd(getResponseLabels(response))
+
+      if (response.body && /^\s*$/.test(response.body)) return {}
       return response.body
-    } catch (error) {
-      // Horrible kludge to handle services returning ' ' as body
-      const response = error.response
-      if (response && response.statusCode < 300) {
-        if (response.body && !response.body.trim()) {
-          requestMetricsEnd(getResponseLabels(response))
+    } catch (e) {
+      const {response = {}} = e
+      if (response.statusCode < 300) {
+        if (response.body && /^\s*$/.test(response.body)) {
+          if (requestMetricsEnd) requestMetricsEnd(getResponseLabels(response))
           return {}
         }
+
+        return response.body
       }
-      apiMetricsEnd(getResponseLabels(error))
-      if (logger) {
-        logError('API', error)
-      }
-      client.handleRequestError(error)
+
+      apiMetricsEnd(getResponseLabels(e))
+
+      if (logger) logError('API', e)
+
+      this.handleRequestError(e)
     }
@@ -493,3 +539,3 @@ }
    *
-   * @param {object} err
+   * @param {object} e
    * Error returned by Request
@@ -501,28 +547,31 @@ *
    **/
-  handleRequestError (err) {
+  handleRequestError (e) {
     // rethrow error if already client error
-    if (err.name === this.ErrorClass.name) {
-      throw err
-    }
-    if (err.body) {
-      if (typeof err.body === 'object') {
-        err.error = err.body
-      }
-    }
-    const {statusCode} = err
+    if (e instanceof this.ErrorClass) throw e
+
+    // adjust
+    if ((e.body || false) instanceof Object) e.error = e.body
+
+    const {
+      statusCode
+    } = e
+
     if (statusCode) {
-      if (statusCode === 404) {
+      if (statusCode > 400 && statusCode < 500) {
         // Data does not exist - ie. expired
-        this.throwRequestError(404)
+        this.throwRequestError(statusCode)
       } else {
-        let message
-        if (err.error) {
-          message = err.error.name || err.error.code || 'EUNSPECIFIED'
+        if (e.error) {
+          // Handle errors which have an error object
+          const message = e.error.name || e.error.code || 'EUNSPECIFIED'
+          this.throwRequestError(statusCode, message)
+        } else {
+          // Handle errors which have no error object
+          const message = e.code || 'ENOERROR'
+          this.throwRequestError(statusCode, message)
         }
-        this.throwRequestError(statusCode, message)
       }
-    } else if (err.error) {
+    } else if (e.error) {
       // Handle errors which have an error object
-      const errorObj = err.error
-      const message = errorObj.name || errorObj.code || 'EUNSPECIFIED'
+      const message = e.error.name || e.error.code || 'EUNSPECIFIED'
       const statusCode = getErrorStatusCode(message)
@@ -532,3 +581,3 @@ this.throwRequestError(statusCode, message)
       // Handle errors which have no error object
-      const message = err.code || 'ENOERROR'
+      const message = e.code || 'ENOERROR'
       const statusCode = getErrorStatusCode(message)
@@ -535,0 +584,0 @@ this.throwRequestError(statusCode, message)

package.json

 {
   "name": "@ministryofjustice/fb-jwt-client-node",
-  "version": "0.0.29",
+  "version": "0.0.30",
   "description": "Form Builder JSON Web Token Client (Node)",
   "main": "lib/fb-jwt-client.js",
   "scripts": {
-    "lint": "eslint lib",
-    "lint:fix": "eslint lib --fix",
-    "test": "tap --reporter spec"
+    "lint": "eslint lib test",
+    "lint:fix": "eslint lib test --fix",
+    "test": "mocha test --recursive"
   },
@@ -20,3 +20,3 @@ "author": "Form Builder Team <[e-mail]>",
     "aes256": "^1.0.4",
-    "got": "^9.6.0",
+    "got": "^10.0.1",
     "jsonwebtoken": "^8.5.1",
@@ -27,7 +27,13 @@ "path-to-regexp": "^6.1.0"
     "@ministryofjustice/eslint-config-fb": "^1.1.2",
+    "@ministryofjustice/module-alias": "^1.0.8",
+    "chai": "^4.2.0",
+    "mocha": "^6.2.2",
     "nock": "^11.6.0",
+    "proxyquire": "^2.1.3",
     "sinon": "^7.5.0",
-    "tap": "^14.8.2",
-    "tape": "^4.11.0"
+    "sinon-chai": "^3.3.0"
+  },
+  "_moduleAliases": {
+    "~/fb-jwt-client-node": "./lib"
   }
 }

New alerts

Major refactor

Supply chain risk

Package has recently undergone a major refactor. It may be unstable or indicate significant internal changes. Use caution when updating to versions that include significant changes.

Found 1 instance in 1 package

Fixed alerts

New author

Supply chain risk

A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

78827

increased by49.3%

Number of package files

11

increased by10%

Lines of code

2017

increased by50.86%

Worsened metrics

Dev dependency count

8

increased by60%

Dependency changes

• + Added@sindresorhus/is@2.1.1(transitive)

• + Added@szmarczak/http-timer@4.0.6(transitive)

• + Added@types/cacheable-request@6.0.3(transitive)

• + Added@types/http-cache-semantics@4.0.4(transitive)

• + Added@types/keyv@3.1.4(transitive)

• + Added@types/node@22.13.4(transitive)

• + Added@types/responselike@1.0.3(transitive)

• + Addedcacheable-lookup@2.0.1(transitive)

• + Addedcacheable-request@7.0.4(transitive)

• + Addeddecompress-response@5.0.0(transitive)

• + Addeddefer-to-connect@2.0.1(transitive)

• + Addedgot@10.7.0(transitive)

• + Addedjson-buffer@3.0.1(transitive)

• + Addedkeyv@4.5.4(transitive)

• + Addedmimic-response@2.1.0(transitive)

• + Addednormalize-url@6.1.0(transitive)

• + Addedp-cancelable@2.1.1(transitive)

• + Addedp-event@4.2.0(transitive)

• + Addedp-finally@1.0.0(transitive)

• + Addedp-timeout@3.2.0(transitive)

• + Addedresponselike@2.0.1(transitive)

• + Addedto-readable-stream@2.1.0(transitive)

• + Addedtype-fest@0.10.0(transitive)

• + Addedundici-types@6.20.0(transitive)

• - Removed@sindresorhus/is@0.14.0(transitive)

• - Removed@szmarczak/http-timer@1.1.2(transitive)

• - Removedcacheable-request@6.1.0(transitive)

• - Removeddecompress-response@3.3.0(transitive)

• - Removeddefer-to-connect@1.1.3(transitive)

• - Removedget-stream@4.1.0(transitive)

• - Removedgot@9.6.0(transitive)

• - Removedjson-buffer@3.0.0(transitive)

• - Removedkeyv@3.1.0(transitive)

• - Removedlowercase-keys@1.0.1(transitive)

• - Removednormalize-url@4.5.1(transitive)

• - Removedp-cancelable@1.1.0(transitive)

• - Removedprepend-http@2.0.0(transitive)

• - Removedresponselike@1.0.2(transitive)

• - Removedto-readable-stream@1.0.0(transitive)

• - Removedurl-parse-lax@3.0.0(transitive)

• Updatedgot@^10.0.1