From 875bc914d4b2365e8ca61157c1cb749c880d1d59 Mon Sep 17 00:00:00 2001 From: Mihir Gupta <37367148+mihirgupta0900@users.noreply.github.com> Date: Mon, 7 Jun 2021 17:20:23 +0530 Subject: [PATCH] Feature/webhook verification (#2) --- src/api-endpoints.ts | 44 +++++++++++ src/coinvise.ts | 145 +++++++++++++++++++++++++++++------- src/helpers.ts | 51 +++++++++++++ src/interfaces/api-types.ts | 1 + 4 files changed, 215 insertions(+), 26 deletions(-) diff --git a/src/api-endpoints.ts b/src/api-endpoints.ts index 6f3bc02..e95a597 100644 --- a/src/api-endpoints.ts +++ b/src/api-endpoints.ts @@ -55,3 +55,47 @@ interface Endpoint { bodyParams: string[]; path: () => string; } + +export type VerifyWebhookParams = { + /** + * Raw text body payload received from Coinvise. + */ + payload: string | Buffer; + + /** + * Value of the `x-coinvise-webhook-signature` header from Coinvise. + * Typically a string. + * + * Note that this is typed to accept an array of strings + * so that it works seamlessly with express's types, + * but will throw if an array is passed in practice + * since express should never return this header as an array, + * only a string. + */ + header: string | Buffer | Array; + + /** + * Your Webhook Signing Secret for this endpoint (e.g., 'whsec_...'). + */ + secret: string; + + /** + * Seconds of tolerance on timestamps. + */ + tolerance?: number; +}; + +export type GenerateWebhookSecretResponse = { + data: { + secret: string; + }; + message: APIMessage.GENERATED; +}; + +export const generateWebhookSecret: Endpoint = { + method: 'GET', + pathParams: [], + queryParams: [], + bodyParams: [], + path: () => 'users/webhook/generate', +}; diff --git a/src/coinvise.ts b/src/coinvise.ts index 918ac14..e3666e1 100644 --- a/src/coinvise.ts +++ b/src/coinvise.ts @@ -1,16 +1,18 @@ import crypto from 'crypto'; -import got, { Agents as GotAgents, Got } from 'got'; -import { Agent } from 'http'; -import { URL } from 'url'; +import got, { Got } from 'got'; import { changeWebhookUrl, ChangeWebhookUrlParams, ChangeWebhookUrlResponse, + generateWebhookSecret, + GenerateWebhookSecretResponse, mintToken, MintTokenParams, MintTokenResponse, + VerifyWebhookParams, } from './api-endpoints'; import { buildRequestError, HTTPResponseError } from './errors'; +import { computeSignature, parseHeader, secureCompare } from './helpers'; import { AuthHeaders } from './interfaces/auth.interface'; import { ClientOptions, @@ -56,7 +58,7 @@ export default class Coinvise { 'user-agent': 'coinvise-client/0.1.0', }, retry: 0, - agent: makeAgentOption(prefixUrl, options?.agent), + // agent: makeAgentOption(prefixUrl, options?.agent), }); } @@ -91,8 +93,8 @@ export default class Coinvise { const response = await this.#got(path, { method, - searchParams: query, - json, + ...(query && { searchParams: query }), + ...(json && { json }), headers, }).json(); @@ -147,6 +149,97 @@ export default class Coinvise { body: args, }); }, + + /** + * Generates webhook secret that is used to verify the integrity of the data sent from Coinvise + */ + generateSecret: async (): Promise => { + const res = await this.request({ + path: generateWebhookSecret.path(), + method: generateWebhookSecret.method, + }); + return res.data.secret; + }, + + /** + * Verifies webhook from the given body and signature + */ + verify: (args: VerifyWebhookParams): boolean => { + const payload = Buffer.isBuffer(args.payload) + ? args.payload.toString('utf8') + : args.payload; + + // Express's type for `Request#headers` is `string | []string` + // which is because the `set-cookie` header is an array, + // but no other headers are an array (docs: https://nodejs.org/api/http.html#http_message_headers) + // (Express's Request class is an extension of http.IncomingMessage, and doesn't appear to be relevantly modified: https://github.com/expressjs/express/blob/master/lib/request.js#L31) + if (Array.isArray(args.header)) { + throw new Error( + 'Unexpected: An array was passed as a header, which should not be possible for the stripe-signature header.' + ); + } + + const header = Buffer.isBuffer(args.header) + ? args.header.toString('utf8') + : args.header; + + const details = parseHeader(header); + + if (!details || details.timestamp === -1) { + this.log( + LogLevel.WARN, + 'Unable to extract timestamp and signatures from header', + { + header, + payload, + } + ); + return false; + } + + if (!details.signature) { + this.log(LogLevel.WARN, 'No signatures found', { + header, + payload, + }); + return false; + } + + const expectedSignature = computeSignature( + `${details.timestamp}.${payload}`, + args.secret + ); + + const signatureFound = secureCompare( + details.signature, + expectedSignature + ); + + if (!signatureFound) { + this.log( + LogLevel.WARN, + 'No signatures found matching the expected signature for payload', + { + header, + payload, + } + ); + return false; + } + + const timestampAge = Math.floor(Date.now() / 1000) - details.timestamp; + + const tolerance = args.tolerance ?? 18000; + if (tolerance > 0 && timestampAge > tolerance) { + this.log(LogLevel.WARN, 'Timestamp outside the tolerance zone', { + header, + payload, + }); + return false; + } + + return true; + }, }; /** @@ -206,26 +299,26 @@ export default class Coinvise { } } -function makeAgentOption( - prefixUrl: string, - agent: Agent | undefined -): GotAgents | undefined { - if (agent === undefined) { - return undefined; - } - return { - [selectProtocol(prefixUrl)]: agent, - }; -} +// function makeAgentOption( +// prefixUrl: string, +// agent: Agent | undefined +// ): GotAgents | undefined { +// if (agent === undefined) { +// return undefined; +// } +// return { +// [selectProtocol(prefixUrl)]: agent, +// }; +// } -function selectProtocol(prefixUrl: string): 'http' | 'https' { - const url = new URL(prefixUrl); +// function selectProtocol(prefixUrl: string): 'http' | 'https' { +// const url = new URL(prefixUrl); - if (url.protocol === 'https:') { - return 'https'; - } else if (url.protocol === 'http:') { - return 'http'; - } +// if (url.protocol === 'https:') { +// return 'https'; +// } else if (url.protocol === 'http:') { +// return 'http'; +// } - throw new TypeError('baseUrl option must begin with "https://" or "http://"'); -} +// throw new TypeError('baseUrl option must begin with "https://" or "http://"'); +// } diff --git a/src/helpers.ts b/src/helpers.ts index 011e6c5..ffecea0 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -1,3 +1,5 @@ +import crypto from 'crypto'; + // eslint-disable-next-line @typescript-eslint/no-unused-vars export function assertNever(_x: never): never { throw new Error('Unexpected value. Should have been never.'); @@ -14,3 +16,52 @@ export function pick( export function isObject(o: unknown): o is Record { return typeof o === 'object' && o !== null; } + +export function parseHeader(header: string): { + timestamp: number; + signature: string; +} | null { + if (typeof header !== 'string') { + return null; + } + + const headerArr = header.split('|'); + return { + timestamp: Number(headerArr[0] ?? '-1'), + signature: headerArr[1] ?? '', + }; +} + +export function computeSignature(payload: string, secret: string): string { + return crypto + .createHmac('sha256', secret) + .update(payload, 'utf8') + .digest('hex'); +} + +/** + * Secure compare, from https://github.com/freewil/scmp + */ +export function secureCompare(a: string, b: string): boolean { + const aBuff: Buffer = Buffer.from(a); + const bBuff: Buffer = Buffer.from(b); + + // return early here if buffer lengths are not equal since timingSafeEqual + // will throw if buffer lengths are not equal + if (aBuff.length !== bBuff.length) { + return false; + } + + // use crypto.timingSafeEqual if available (since Node.js v6.6.0), + // otherwise use our own scmp-internal function. + if (crypto.timingSafeEqual) { + return crypto.timingSafeEqual(aBuff, bBuff); + } + + let result = 0; + + for (let i = 0; i < aBuff.length; ++i) { + result |= (aBuff[i] ?? 0) ^ (bBuff[i] ?? 0); + } + return result === 0; +} diff --git a/src/interfaces/api-types.ts b/src/interfaces/api-types.ts index 40e0465..3d55cc6 100644 --- a/src/interfaces/api-types.ts +++ b/src/interfaces/api-types.ts @@ -1,4 +1,5 @@ export enum APIMessage { CREATED = 'created', UPDATED = 'updated', + GENERATED = 'generated', }