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

Add a CryptoProvider interface and NodeCryptoProvider implementation. #1237

Merged
merged 1 commit into from
Sep 13, 2021
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
38 changes: 24 additions & 14 deletions lib/Webhooks.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
'use strict';

const crypto = require('crypto');

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

const Webhook = {
DEFAULT_TOLERANCE: 300, // 5 minutes

constructEvent(payload, header, secret, tolerance) {
constructEvent(payload, header, secret, tolerance, cryptoProvider) {
this.signature.verifyHeader(
payload,
header,
secret,
tolerance || Webhook.DEFAULT_TOLERANCE
tolerance || Webhook.DEFAULT_TOLERANCE,
cryptoProvider
);

const jsonPayload = JSON.parse(payload);
Expand All @@ -29,6 +28,7 @@ const Webhook = {
* @property {string} secret - Stripe webhook secret 'whsec_...'
* @property {string} scheme - Version of API to hit. Defaults to 'v1'.
* @property {string} signature - Computed webhook signature
* @property {CryptoProvider} cryptoProvider - Crypto provider to use for computing the signature if none was provided. Defaults to NodeCryptoProvider.
*/
generateTestHeaderString: function(opts) {
if (!opts) {
Expand All @@ -41,9 +41,11 @@ const Webhook = {
Math.floor(opts.timestamp) || Math.floor(Date.now() / 1000);
opts.scheme = opts.scheme || signature.EXPECTED_SCHEME;

opts.cryptoProvider = opts.cryptoProvider || getNodeCryptoProvider();

opts.signature =
opts.signature ||
signature._computeSignature(
opts.cryptoProvider.computeHMACSignature(
opts.timestamp + '.' + opts.payload,
opts.secret
);
Expand All @@ -60,14 +62,7 @@ const Webhook = {
const signature = {
EXPECTED_SCHEME: 'v1',

_computeSignature: (payload, secret) => {
return crypto
.createHmac('sha256', secret)
.update(payload, 'utf8')
.digest('hex');
},

verifyHeader(payload, header, secret, tolerance) {
verifyHeader(payload, header, secret, tolerance, cryptoProvider) {
payload = Buffer.isBuffer(payload) ? payload.toString('utf8') : payload;

// Express's type for `Request#headers` is `string | []string`
Expand Down Expand Up @@ -104,7 +99,8 @@ const signature = {
});
}

const expectedSignature = this._computeSignature(
cryptoProvider = cryptoProvider || getNodeCryptoProvider();
const expectedSignature = cryptoProvider.computeHMACSignature(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be evil, but for async support:

- const expectedSignature = cryptoProvider.computeHMACSignature(
-    ...
- );
- // Do a bunch of stuff with expectedSignature
+ const compare = (expectedSignature) => {
+   // Do a bunch of stuff with expectedSignature
+ };
+ const expectedSignature = cryptoProvider.computeHMACSignature(
+   ...
+ );
+ if (expectedSignature && typeof expectedSignature.then === 'function') {
+   return expectedSignature.then(compare);
+ } else {
+   return compare(expectedSignature);
+ }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I think this could potentially simplify the implementation over having two separate methods... let's think about it and see how the need comes up? Either change would require updating the types though I imagine changing constructEvent's return type from Stripe.Event to Stripe.Event | Promise<Stripe.Event> wouldn't technically be backwards compatible?

Think we can merge this and handle async in a separate PR.

`${details.timestamp}.${payload}`,
secret
);
Expand Down Expand Up @@ -168,6 +164,20 @@ function parseHeader(header, scheme) {
);
}

let webhooksNodeCryptoProviderInstance = null;

/**
* Lazily instantiate a NodeCryptoProvider instance. This is a stateless object
* so a singleton can be used here.
*/
function getNodeCryptoProvider() {
if (!webhooksNodeCryptoProviderInstance) {
const NodeCryptoProvider = require('./crypto/NodeCryptoProvider');
webhooksNodeCryptoProviderInstance = new NodeCryptoProvider();
}
return webhooksNodeCryptoProviderInstance;
}

Webhook.signature = signature;

module.exports = Webhook;
21 changes: 21 additions & 0 deletions lib/crypto/CryptoProvider.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

/**
* Interface encapsulating the various crypto computations used by the library,
* allowing pluggable underlying crypto implementations.
*/
class CryptoProvider {
/**
* Computes a SHA-256 HMAC given a secret and a payload (encoded in UTF-8).
* The output HMAC should be encoded in hexadecimal.
*
* Sample values for implementations:
* - computeHMACSignature('', 'test_secret') => 'f7f9bd47fb987337b5796fdc1fdb9ba221d0d5396814bfcaf9521f43fd8927fd'
* - computeHMACSignature('\ud83d\ude00', 'test_secret') => '837da296d05c4fe31f61d5d7ead035099d9585a5bcde87de952012a78f0b0c43
*/
computeHMACSignature(payload, secret) {
throw new Error('computeHMACSignature not implemented.');
}
}

module.exports = CryptoProvider;
20 changes: 20 additions & 0 deletions lib/crypto/NodeCryptoProvider.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict';

const crypto = require('crypto');

const CryptoProvider = require('./CryptoProvider');

/**
* `CryptoProvider which uses the Node `crypto` package for its computations.
*/
class NodeCryptoProvider extends CryptoProvider {
/** @override */
computeHMACSignature(payload, secret) {
return crypto
.createHmac('sha256', secret)
.update(payload, 'utf8')
.digest('hex');
}
}

module.exports = NodeCryptoProvider;
3 changes: 3 additions & 0 deletions lib/stripe.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ const {HttpClient, HttpClientResponse} = require('./net/HttpClient');
Stripe.HttpClient = HttpClient;
Stripe.HttpClientResponse = HttpClientResponse;

const CryptoProvider = require('./crypto/CryptoProvider');
Stripe.CryptoProvider = CryptoProvider;

function Stripe(key, config = {}) {
if (!(this instanceof Stripe)) {
return new Stripe(key, config);
Expand Down
64 changes: 63 additions & 1 deletion test/Webhook.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const stripe = require('../testUtils').getSpyableStripe();
const {getSpyableStripe, FakeCryptoProvider} = require('../testUtils');
const stripe = getSpyableStripe();
const expect = require('chai').expect;

const EVENT_PAYLOAD = {
Expand Down Expand Up @@ -86,6 +87,24 @@ describe('Webhooks', () => {
'Unexpected: An array was passed as a header, which should not be possible for the stripe-signature header.'
);
});

it('should invoke a custom CryptoProvider', () => {
const header = stripe.webhooks.generateTestHeaderString({
payload: EVENT_PAYLOAD_STRING,
secret: SECRET,
signature: 'fake signature',
});

const event = stripe.webhooks.constructEvent(
EVENT_PAYLOAD_STRING,
header,
SECRET,
undefined,
new FakeCryptoProvider()
);

expect(event.id).to.equal(EVENT_PAYLOAD.id);
});
});

describe('.verifySignatureHeader', () => {
Expand Down Expand Up @@ -254,5 +273,48 @@ describe('Webhooks', () => {
)
).to.equal(true);
});

describe('custom CryptoProvider', () => {
const cryptoProvider = new FakeCryptoProvider();

it('should use the provider to compute a signature (mismatch)', () => {
const header = stripe.webhooks.generateTestHeaderString({
payload: EVENT_PAYLOAD_STRING,
secret: SECRET,
signature: 'different fake signature',
timestamp: 123,
});

expect(() => {
stripe.webhooks.signature.verifyHeader(
EVENT_PAYLOAD_STRING,
header,
SECRET,
undefined,
cryptoProvider
);
}).to.throw(
/No signatures found matching the expected signature for payload/
);
});
it('should use the provider to compute a signature (success)', () => {
const header = stripe.webhooks.generateTestHeaderString({
payload: EVENT_PAYLOAD_STRING,
secret: SECRET,
signature: 'fake signature',
timestamp: 123,
});

expect(
stripe.webhooks.signature.verifyHeader(
EVENT_PAYLOAD_STRING,
header,
SECRET,
undefined,
cryptoProvider
)
).to.equal(true);
});
});
});
});
9 changes: 9 additions & 0 deletions test/crypto/NodeCryptoProvider.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict';

const NodeCryptoProvider = require('../../lib/crypto/NodeCryptoProvider');

const {createCryptoProviderTestSuite} = require('./helpers');

describe('NodeCryptoProvider', () => {
createCryptoProviderTestSuite(new NodeCryptoProvider());
});
42 changes: 42 additions & 0 deletions test/crypto/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';

const expect = require('chai').expect;

const SECRET = 'test_secret';

/**
* Test runner which runs a common set of tests for a given CryptoProvider to
* make sure it satisfies the expected contract.
*/
const createCryptoProviderTestSuite = (cryptoProvider) => {
describe('CryptoProviderTestSuite', () => {
describe('computeHMACSignature', () => {
it('empty payload', () => {
expect(cryptoProvider.computeHMACSignature('', SECRET)).to.equal(
'f7f9bd47fb987337b5796fdc1fdb9ba221d0d5396814bfcaf9521f43fd8927fd'
);
});

it('sample payload', () => {
expect(
cryptoProvider.computeHMACSignature(
JSON.stringify({obj1: 'hello', obj2: 'world'}),
SECRET
)
).to.equal(
'bebb1a643997f419b315ddba19e6f5411e1ce7f810ba6d3617ce72823092f363'
);
});

it('payload with utf-8', () => {
expect(
cryptoProvider.computeHMACSignature('\ud83d\ude00', SECRET)
).to.equal(
'837da296d05c4fe31f61d5d7ead035099d9585a5bcde87de952012a78f0b0c43'
);
});
});
});
};

module.exports = {createCryptoProviderTestSuite};
7 changes: 7 additions & 0 deletions testUtils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require('chai').use(require('chai-as-promised'));

const http = require('http');

const CryptoProvider = require('../lib/crypto/CryptoProvider');
const ResourceNamespace = require('../lib/ResourceNamespace').ResourceNamespace;

const testingHttpAgent = new http.Agent({keepAlive: false});
Expand Down Expand Up @@ -210,4 +211,10 @@ const utils = (module.exports = {
return false;
}
},

FakeCryptoProvider: class extends CryptoProvider {
computeHMACSignature(payload, secret) {
return 'fake signature';
}
},
});
1 change: 1 addition & 0 deletions types/2020-08-27/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// File generated from our OpenAPI spec

///<reference path='../lib.d.ts' />
///<reference path='../crypto/crypto.d.ts' />
///<reference path='../net/net.d.ts' />
///<reference path='../shared.d.ts' />
///<reference path='../Errors.d.ts' />
Expand Down
16 changes: 14 additions & 2 deletions types/Webhooks.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ declare module 'stripe' {
/**
* Seconds of tolerance on timestamps.
*/
tolerance?: number
tolerance?: number,

/**
* Optional CryptoProvider to use for computing HMAC signatures.
*/
cryptoProvider?: CryptoProvider
): Stripe.Event;

/**
Expand Down Expand Up @@ -66,6 +71,12 @@ declare module 'stripe' {
* Computed webhook signature.
*/
signature?: string;

/**
* Optional CryptoProvider to use for computing HMAC signatures, if no
* signature is given.
*/
cryptoProvider?: CryptoProvider;
}): string;

signature: Signature;
Expand All @@ -79,7 +90,8 @@ declare module 'stripe' {
payload: string,
header: string,
secret: string,
tolerance?: number
tolerance?: number,
cryptoProvider?: CryptoProvider
): void;
parseHeader(
header: string,
Expand Down
19 changes: 19 additions & 0 deletions types/crypto/crypto.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
declare module 'stripe' {
namespace Stripe {
/**
* Interface encapsulating the various crypto computations used by the library,
* allowing pluggable underlying crypto implementations.
*/
export interface CryptoProvider {
/**
* Computes a SHA-256 HMAC given a secret and a payload (encoded in UTF-8).
* The output HMAC should be encoded in hexadecimal.
*
* Sample values for implementations:
* - computeHMACSignature('', 'test_secret') => 'f7f9bd47fb987337b5796fdc1fdb9ba221d0d5396814bfcaf9521f43fd8927fd'
* - computeHMACSignature('\ud83d\ude00', 'test_secret') => '837da296d05c4fe31f61d5d7ead035099d9585a5bcde87de952012a78f0b0c43
*/
computeHMACSignature: (payload: string, secret: string) => string;
}
}
}