Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept an array of strings or buffers as the header argument for webhooks.constructEvent #753

Merged
merged 3 commits into from
Jan 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions lib/StripeResource.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@ const https = require('https');
const path = require('path');

const utils = require('./utils');
const Error = require('./Error');
const {
StripeConnectionError,
StripeAuthenticationError,
StripePermissionError,
StripeRateLimitError,
StripeError,
StripeAPIError,
} = require('./Error');

const defaultHttpAgent = new http.Agent({keepAlive: true});
const defaultHttpsAgent = new https.Agent({keepAlive: true});
Expand Down Expand Up @@ -96,7 +103,7 @@ StripeResource.prototype = {

callback.call(
this,
new Error.StripeConnectionError({
new StripeConnectionError({
message: `Request aborted due to timeout being reached (${timeout}ms)`,
detail: timeoutErr,
}),
Expand Down Expand Up @@ -159,20 +166,20 @@ StripeResource.prototype = {
response.error.requestId = res.requestId;

if (res.statusCode === 401) {
err = new Error.StripeAuthenticationError(response.error);
err = new StripeAuthenticationError(response.error);
} else if (res.statusCode === 403) {
err = new Error.StripePermissionError(response.error);
err = new StripePermissionError(response.error);
} else if (res.statusCode === 429) {
err = new Error.StripeRateLimitError(response.error);
err = new StripeRateLimitError(response.error);
} else {
err = Error.StripeError.generate(response.error);
err = StripeError.generate(response.error);
}
return callback.call(this, err, null);
}
} catch (e) {
return callback.call(
this,
new Error.StripeAPIError({
new StripeAPIError({
message: 'Invalid JSON received from the Stripe API',
response,
exception: e,
Expand Down Expand Up @@ -209,7 +216,7 @@ StripeResource.prototype = {
}
callback.call(
this,
new Error.StripeConnectionError({
new StripeConnectionError({
message: this._generateConnectionErrorMessage(requestRetries),
detail: error,
}),
Expand Down
23 changes: 17 additions & 6 deletions lib/Webhooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const crypto = require('crypto');

const utils = require('./utils');
const Error = require('./Error');
const {StripeError, StripeSignatureVerificationError} = require('./Error');

const Webhook = {
DEFAULT_TOLERANCE: 300, // 5 minutes
Expand Down Expand Up @@ -32,7 +32,7 @@ const Webhook = {
*/
generateTestHeaderString: function(opts) {
if (!opts) {
throw new Error.StripeError({
throw new StripeError({
message: 'Options are required',
});
}
Expand Down Expand Up @@ -69,12 +69,23 @@ const signature = {

verifyHeader(payload, header, secret, tolerance) {
payload = Buffer.isBuffer(payload) ? payload.toString('utf8') : 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(header)) {
throw new Error(
'Unexpected: An array was passed as a header, which should not be possible for the stripe-signature header.'
);
}

header = Buffer.isBuffer(header) ? header.toString('utf8') : header;

const details = parseHeader(header, this.EXPECTED_SCHEME);

if (!details || details.timestamp === -1) {
throw new Error.StripeSignatureVerificationError({
throw new StripeSignatureVerificationError({
message: 'Unable to extract timestamp and signatures from header',
detail: {
header,
Expand All @@ -84,7 +95,7 @@ const signature = {
}

if (!details.signatures.length) {
throw new Error.StripeSignatureVerificationError({
throw new StripeSignatureVerificationError({
message: 'No signatures found with expected scheme',
detail: {
header,
Expand All @@ -103,7 +114,7 @@ const signature = {
).length;

if (!signatureFound) {
throw new Error.StripeSignatureVerificationError({
throw new StripeSignatureVerificationError({
message:
'No signatures found matching the expected signature for payload.' +
' Are you passing the raw request body you received from Stripe?' +
Expand All @@ -118,7 +129,7 @@ const signature = {
const timestampAge = Math.floor(Date.now() / 1000) - details.timestamp;

if (tolerance > 0 && timestampAge > tolerance) {
throw new Error.StripeSignatureVerificationError({
throw new StripeSignatureVerificationError({
message: 'Timestamp outside the tolerance zone',
detail: {
header,
Expand Down
13 changes: 13 additions & 0 deletions test/Webhook.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ describe('Webhooks', () => {
stripe.webhooks.constructEvent(EVENT_PAYLOAD_STRING, header, SECRET);
}).to.throw(/Unable to extract timestamp and signatures from header/);
});

it('should error if you pass a signature which is an array, even though our types say you can', () => {
const header = stripe.webhooks.generateTestHeaderString({
payload: EVENT_PAYLOAD_STRING,
secret: SECRET,
});

expect(() => {
stripe.webhooks.constructEvent(EVENT_PAYLOAD_STRING, [header], SECRET);
}).to.throw(
'Unexpected: An array was passed as a header, which should not be possible for the stripe-signature header.'
);
});
});

describe('.verifySignatureHeader', () => {
Expand Down
9 changes: 8 additions & 1 deletion types/Webhooks.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,15 @@ declare module 'stripe' {

/**
* Value of the `stripe-signature` header from Stripe.
* 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,
header: string | Buffer | Array<string>,

/**
* Your Webhook Signing Secret for this endpoint (e.g., 'whsec_...').
Expand Down