Skip to content

Commit

Permalink
fix!: changed default header to authorization
Browse files Browse the repository at this point in the history 
changing options to interface
moving order of exports
validateArguments now throws TypeError directly
changed to req.get(header) for better mixed case handling
  • Loading branch information
@connorjburton
connorjburton committed Jul 11, 2021
1 parent ef85f27 commit 46e1f62
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 40 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
33 changes: 14 additions & 19 deletions src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Options> = {}): 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
Expand All @@ -75,8 +70,8 @@ export function HMAC(secret: string, options: Partial<Options> = {}): 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)) {
Expand Down
14 changes: 7 additions & 7 deletions src/validateArguments.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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})`);
}
}
2 changes: 1 addition & 1 deletion tests/e2e.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`
}]
}
});
Expand Down
24 changes: 12 additions & 12 deletions tests/unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -19,7 +19,7 @@ type Spies = {
function mockedRequest(override?: MockRequest): Partial<Request> {
return {
headers: {
authentication: 'HMAC 1573504737300:76251c6323fbf6355f23816a4c2e12edfd10672517104763ab1b10f078277f86'
authorization: 'HMAC 1573504737300:76251c6323fbf6355f23816a4c2e12edfd10672517104763ab1b10f078277f86'
},
method: 'POST',
originalUrl: '/api/order',
Expand Down Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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);
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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', () => {
Expand Down

0 comments on commit 46e1f62

Please sign in to comment.