From 46e1f62d216323bcd9e7fb7248ffeda4304262b7 Mon Sep 17 00:00:00 2001 From: Connor Burton Date: Sun, 11 Jul 2021 15:05:58 +0100 Subject: [PATCH] fix!: changed default header to authorization changing options to interface moving order of exports validateArguments now throws TypeError directly changed to req.get(header) for better mixed case handling --- README.md | 2 +- src/index.ts | 33 ++++++++++++++------------------- src/validateArguments.ts | 14 +++++++------- tests/e2e.test.ts | 2 +- tests/unit.test.ts | 24 ++++++++++++------------ 5 files changed, 35 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index e3f562a..97defca 100644 --- a/README.md +++ b/README.md @@ -177,7 +177,7 @@ You can run your own benchmarks by checking out the package and running `yarn be import { HMAC as hmac } from 'hmac-auth-express'; ``` -*Why is there no default export?* It seems to be non-trivial to export a default that has consistent behaviour between CommonJS & ECMASCript, the example below shows the behavioural differences when exporting a default from Typescript. +*Why is there no default export?* It seems to be non-trivial to export a default that has consistent behaviour between CommonJS & ECMASCript, the example below shows the behavioural differences when exporting a default from TypeScript. ```javascript const HMAC = require('hmac-auth-express').default; diff --git a/src/index.ts b/src/index.ts index 7743e0f..88abf55 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,7 +5,9 @@ import stringToBuffer from './stringToBuffer'; import validateArguments from './validateArguments'; import AuthError from './errors'; -export type Options = { +export { AuthError }; + +export interface Options { algorithm: string; identifier: string; header: string; @@ -16,53 +18,46 @@ export type Options = { const defaults: Options = { algorithm: 'sha256', identifier: 'HMAC', - header: 'authentication', + header: 'authorization', maxInterval: 60 * 5, minInterval: 0 } -export { AuthError }; - export function HMAC(secret: string, options: Partial = {}): RequestHandler { const mergedOpts: Options = { ...defaults, ...options }; - const error: TypeError | void = validateArguments(secret, mergedOpts); - if (error instanceof TypeError) { - throw error; - } + validateArguments(secret, mergedOpts); return function(request: Request, _, next: NextFunction): void { - if (!(mergedOpts.header in request.headers)) { + const header = request.get(mergedOpts.header); + if (typeof header !== 'string') { return next(new AuthError(`Header provided not in sent headers. Expected ${mergedOpts.header} but not found in request.headers`)); } - // see https://github.com/microsoft/TypeScript/issues/41612 - // for why we can't do request.headers[options.header] in the conditional - const header: string | string[] | undefined = request.headers[mergedOpts.header]; - if (typeof header !== 'string' || typeof mergedOpts.identifier !== 'string' || !header.startsWith(mergedOpts.identifier)) { + if (typeof mergedOpts.identifier !== 'string' || !header.startsWith(mergedOpts.identifier)) { return next(new AuthError(`Header did not start with correct identifier. Expected ${mergedOpts.identifier} but not found in options.header`)); } const unixRegex = /(\d{1,13}):/; - const unixMatch: RegExpExecArray | null = unixRegex.exec(header); + const unixMatch = unixRegex.exec(header); if (!Array.isArray(unixMatch) || unixMatch.length !== 2 || typeof unixMatch[1] !== 'string') { return next(new AuthError('Unix timestamp was not present in header')); } // is the unix timestamp difference to current timestamp larger than maxInterval or smaller than minInterval - const timeDiff: number = Math.floor(Date.now() / 1000) - Math.floor(parseInt(unixMatch[1], 10) / 1000); + const timeDiff = Math.floor(Date.now() / 1000) - Math.floor(parseInt(unixMatch[1], 10) / 1000); if (timeDiff > mergedOpts.maxInterval || (timeDiff * -1) > mergedOpts.minInterval) { return next(new AuthError('Time difference between generated and requested time is too great')); } // check HMAC digest exists in header const hashRegex = /:(.{1,}$)/; - const hashMatch: RegExpExecArray | null = hashRegex.exec(header); + const hashMatch = hashRegex.exec(header); if (!Array.isArray(hashMatch) || hashMatch.length !== 2 || typeof hashMatch[1] !== 'string') { return next(new AuthError('HMAC digest was not present in header')); } - const hmac: crypto.Hmac = crypto.createHmac(mergedOpts.algorithm, secret); + const hmac = crypto.createHmac(mergedOpts.algorithm, secret); hmac.update(unixMatch[1]); // add timestamp provided in header to make sure it hasn't been changed hmac.update(request.method); // add verb e.g POST, GET @@ -75,8 +70,8 @@ export function HMAC(secret: string, options: Partial = {}): RequestHan hmac.update(hash.digest('hex')); } - const hmacDigest: Buffer = hmac.digest(); // returns Uint8Array buffer - const sourceDigest: Buffer = stringToBuffer(hashMatch[1]); // convert string to buffer + const hmacDigest = hmac.digest(); // returns Uint8Array buffer + const sourceDigest = stringToBuffer(hashMatch[1]); // convert string to buffer // use timing safe check to prevent timing attacks if (hmacDigest.length !== sourceDigest.length || !crypto.timingSafeEqual(hmacDigest, sourceDigest)) { diff --git a/src/validateArguments.ts b/src/validateArguments.ts index 586cfcd..1416a1f 100644 --- a/src/validateArguments.ts +++ b/src/validateArguments.ts @@ -2,28 +2,28 @@ import crypto from 'crypto'; import { Options } from './index'; -export default function(secret: string, options: Options): TypeError | void { +export default function(secret: string, options: Options): void { if (!secret || typeof secret !== 'string') { - return new TypeError(`Invalid value provided for property secret. Expected non-empty string but got '${secret}' (type: ${typeof secret})`); + throw new TypeError(`Invalid value provided for property secret. Expected non-empty string but got '${secret}' (type: ${typeof secret})`); } if (typeof options.algorithm !== 'string' || !crypto.getHashes().includes(options.algorithm)) { - return new TypeError(`Invalid value provided for property options.algorithm. Expected value from crypto.getHashes() but got ${options.algorithm}`); + throw new TypeError(`Invalid value provided for property options.algorithm. Expected value from crypto.getHashes() but got ${options.algorithm} (type: ${typeof options.algorithm})`); } if (!options.identifier || typeof options.identifier !== 'string') { - return new TypeError(`Invalid value provided for property options.identifier. Expected non-empty string but got '${options.identifier}' (type: ${typeof options.identifier})`); + throw new TypeError(`Invalid value provided for property options.identifier. Expected non-empty string but got '${options.identifier}' (type: ${typeof options.identifier})`); } if (!options.header || typeof options.header !== 'string') { - return new TypeError(`Invalid value provided for property options.header. Expected non-empty string but got '${options.header}' (type: ${typeof options.header})`); + throw new TypeError(`Invalid value provided for property options.header. Expected non-empty string but got '${options.header}' (type: ${typeof options.header})`); } if (!options.maxInterval || typeof options.maxInterval !== 'number' || Number.isNaN(options.maxInterval)) { - return new TypeError(`Invalid value provided for property options.maxInterval. Expected number but got '${options.maxInterval}' (type: ${typeof options.maxInterval})`); + throw new TypeError(`Invalid value provided for property options.maxInterval. Expected number but got '${options.maxInterval}' (type: ${typeof options.maxInterval})`); } if (typeof options.minInterval !== 'number' || Number.isNaN(options.minInterval) || options.minInterval < 0) { - return new TypeError(`Invalid value provided for optional property options.minInterval. Expected positive number but got '${options.minInterval}' (type: ${typeof options.minInterval})`); + throw new TypeError(`Invalid value provided for optional property options.minInterval. Expected positive number but got '${options.minInterval}' (type: ${typeof options.minInterval})`); } } \ No newline at end of file diff --git a/tests/e2e.test.ts b/tests/e2e.test.ts index 78f4f52..13a7700 100644 --- a/tests/e2e.test.ts +++ b/tests/e2e.test.ts @@ -54,7 +54,7 @@ describe('e2e', () => { json: body, hooks: { beforeRequest: [(options: NormalizedOptions) => { - options.headers['authentication'] = `HMAC ${time.toString()}:${generate(options.json, time, options.method, options.url.pathname)}` + options.headers['authorization'] = `HMAC ${time.toString()}:${generate(options.json, time, options.method, options.url.pathname)}` }] } }); diff --git a/tests/unit.test.ts b/tests/unit.test.ts index e1424b1..c004abb 100644 --- a/tests/unit.test.ts +++ b/tests/unit.test.ts @@ -4,8 +4,8 @@ import { Request, Response } from 'express'; import { HMAC, AuthError } from './../src/index.js'; type MockRequest = { - headers: { - authentication?: string + headers?: { + authorization?: string } method?: 'GET' | 'POST', originalUrl?: string, @@ -19,7 +19,7 @@ type Spies = { function mockedRequest(override?: MockRequest): Partial { return { headers: { - authentication: 'HMAC 1573504737300:76251c6323fbf6355f23816a4c2e12edfd10672517104763ab1b10f078277f86' + authorization: 'HMAC 1573504737300:76251c6323fbf6355f23816a4c2e12edfd10672517104763ab1b10f078277f86' }, method: 'POST', originalUrl: '/api/order', @@ -58,7 +58,7 @@ describe('unit', () => { const middleware = HMAC('secret'); - middleware(mockedRequest({ headers: { authentication: 'HMAC 1573504737300:4f1c59c68f09af0790b4531118438ae179689eebc5bb30a8359719e319f70b85' }, body: [1, 2, 3] }) as Request, {} as Response, spies.next); + middleware(mockedRequest({ headers: { authorization: 'HMAC 1573504737300:4f1c59c68f09af0790b4531118438ae179689eebc5bb30a8359719e319f70b85' }, body: [1, 2, 3] }) as Request, {} as Response, spies.next); expect(spies.next).toHaveBeenLastCalledWith(); @@ -71,7 +71,7 @@ describe('unit', () => { const middleware = HMAC('secret', { algorithm: 'ripemd160' }); - middleware(mockedRequest({ headers: { authentication: 'HMAC 1573504737300:b55d3ad0b64e106655871bbe7e0d1f55a1f81f7b' }}) as Request, {} as Response, spies.next); + middleware(mockedRequest({ headers: { authorization: 'HMAC 1573504737300:b55d3ad0b64e106655871bbe7e0d1f55a1f81f7b' }}) as Request, {} as Response, spies.next); expect(spies.next).toHaveBeenLastCalledWith(); @@ -86,7 +86,7 @@ describe('unit', () => { middleware(mockedRequest({ headers: { - authentication: 'HMAC 1573504737300:39f9c6b0ea547d46ac03d4e7b0acd1194c2a06f1037620ba7986f8eb017c98ba' + authorization: 'HMAC 1573504737300:39f9c6b0ea547d46ac03d4e7b0acd1194c2a06f1037620ba7986f8eb017c98ba' }, body: undefined }) as Request, {} as Response, spies.next); @@ -118,7 +118,7 @@ describe('unit', () => { const calledArg = spies.next.mock.calls.pop()[0]; expect(calledArg).toBeInstanceOf(AuthError); - expect(calledArg.message).toBe('Header provided not in sent headers. Expected authentication but not found in request.headers'); + expect(calledArg.message).toBe('Header provided not in sent headers. Expected authorization but not found in request.headers'); }); test('fails hmac on no header with custom header', () => { @@ -134,7 +134,7 @@ describe('unit', () => { test('fails hmac on incorrect identifier', () => { const middleware = HMAC('secret'); - middleware(mockedRequest({ headers: { authentication: 'FOO' } }) as Request, {} as Response, spies.next); + middleware(mockedRequest({ headers: { authorization: 'FOO' } }) as Request, {} as Response, spies.next); const calledArg = spies.next.mock.calls.pop()[0]; expect(calledArg).toBeInstanceOf(AuthError); @@ -144,7 +144,7 @@ describe('unit', () => { test('fails hmac on incorrect identifier with custom identifier', () => { const middleware = HMAC('secret', { identifier: 'BAR' }); - middleware(mockedRequest({ headers: { authentication: 'FOO' } }) as Request, {} as Response, spies.next); + middleware(mockedRequest({ headers: { authorization: 'FOO' } }) as Request, {} as Response, spies.next); const calledArg = spies.next.mock.calls.pop()[0]; expect(calledArg).toBeInstanceOf(AuthError); @@ -154,7 +154,7 @@ describe('unit', () => { test('fails if unix timestamp not found', () => { const middleware = HMAC('secret'); - middleware(mockedRequest({ headers: { authentication: 'HMAC :a2bc3' } }) as Request, {} as Response, spies.next); + middleware(mockedRequest({ headers: { authorization: 'HMAC :a2bc3' } }) as Request, {} as Response, spies.next); const calledArg = spies.next.mock.calls.pop()[0]; expect(calledArg).toBeInstanceOf(AuthError); @@ -240,7 +240,7 @@ describe('unit', () => { const middleware = HMAC('secret'); - middleware(mockedRequest({ headers: { authentication: 'HMAC 1573504737300:' }}) as Request, {} as Response, spies.next); + middleware(mockedRequest({ headers: { authorization: 'HMAC 1573504737300:' }}) as Request, {} as Response, spies.next); const calledArg = spies.next.mock.calls.pop()[0]; expect(calledArg).toBeInstanceOf(AuthError); @@ -258,7 +258,7 @@ describe('unit', () => { }); test('passing incorrect algorithm throws an error', () => { - expect(() => HMAC('secret', { algorithm: 'sha111' })).toThrowError(new TypeError(`Invalid value provided for property options.algorithm. Expected value from crypto.getHashes() but got sha111`)); + expect(() => HMAC('secret', { algorithm: 'sha111' })).toThrowError(new TypeError(`Invalid value provided for property options.algorithm. Expected value from crypto.getHashes() but got sha111 (type: string)`)); }); test('passing incorrect identifier throws an error', () => {