From 44f1c0a7b61adefb9c14919000cab3460a7a67ee Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 28 May 2020 17:58:51 +0200 Subject: [PATCH] Updated members config to respect stripeDirect no-issue If the stripeDirect config value is NOT set / false, then connect token should be used over the API keys if both are present, or else use whichever is present. If the stripeDirect config value is set / true,then API keys should be used and stripe connect token should be completely ignored. The reason for this is that we want to use Stripe Connect as the primary auth method going forward, but we want to keep the direct version availiable should the Ghost Foundation ever dismantle. This will allow people to use all the same features with no dependency on an external service. --- core/server/services/members/config.js | 36 +++++- test/unit/services/members/config_spec.js | 136 ++++++++++++++++++++++ 2 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 test/unit/services/members/config_spec.js diff --git a/core/server/services/members/config.js b/core/server/services/members/config.js index 570e9a0085..a4356cdf9b 100644 --- a/core/server/services/members/config.js +++ b/core/server/services/members/config.js @@ -82,6 +82,33 @@ class MembersConfigProvider { } } + /** + * @function getStripeAPIKeys + * @desc Gets the stripe api keys from settings, respecting the stripeDirect config + * + * @param {string} publicKey - The publicKey to use if stripeDirect is enabled + * @param {string} secretKey - The secretKey to use if stripeDirect is enabled + * + * @returns {{publicKey: string, secretKey: string}} + */ + getStripeAPIKeys(publicKey, secretKey) { + const stripeDirect = this._config.get('stripeDirect'); + const stripeConnectIntegration = this._settingsCache.get('stripe_connect_integration'); + const hasStripeConnectKeys = stripeConnectIntegration.secret_key && stripeConnectIntegration.public_key; + + if (stripeDirect || !hasStripeConnectKeys) { + return { + publicKey, + secretKey + }; + } + + return { + publicKey: stripeConnectIntegration.public_key, + secretKey: stripeConnectIntegration.secret_key + }; + } + getStripePaymentConfig() { const subscriptionSettings = this._settingsCache.get('members_subscription_settings'); @@ -114,9 +141,14 @@ class MembersConfigProvider { const billingCancelUrl = new URL(siteUrl); billingCancelUrl.searchParams.set('stripe', 'billing-update-cancel'); + const stripeApiKeys = this.getStripeAPIKeys( + stripePaymentProcessor.config.public_token, + stripePaymentProcessor.config.secret_token + ); + return { - publicKey: stripePaymentProcessor.config.public_token, - secretKey: stripePaymentProcessor.config.secret_token, + publicKey: stripeApiKeys.publicKey, + secretKey: stripeApiKeys.secretKey, checkoutSuccessUrl: checkoutSuccessUrl.href, checkoutCancelUrl: checkoutCancelUrl.href, billingSuccessUrl: billingSuccessUrl.href, diff --git a/test/unit/services/members/config_spec.js b/test/unit/services/members/config_spec.js new file mode 100644 index 0000000000..d05ac08477 --- /dev/null +++ b/test/unit/services/members/config_spec.js @@ -0,0 +1,136 @@ +const should = require('should'); +const MembersConfigProvider = require('../../../../core/server/services/members/config'); +const urlUtils = require('../../../../core/shared/url-utils'); +const sinon = require('sinon'); + +/** + * @param {object} options + * @param {boolean} options.stripeDirectValue - The value the stripeDirect config property should have + */ +function createConfigMock({stripeDirectValue}) { + return { + get: sinon.stub() + .withArgs('stripeDirect').returns(stripeDirectValue) + }; +} + +/** + * @param {object} options + * @param {boolean} options.setDirect - Whether the "direct" keys should be set + * @param {boolean} options.setConnect - Whether the connect_integration keys should be set + */ + +function createSettingsMock({setDirect, setConnect}) { + const membersSubscriptionSettings = { + fromAddress: 'noreply', + allowSelfSignup: true, + paymentProcessors: [{ + adapter: 'stripe', + config: { + secret_token: setDirect ? 'direct_secret' : null, + public_token: setDirect ? 'direct_public' : null, + product: { + name: 'Test' + }, + plans: [{ + name: 'Monthly', + currency: 'usd', + interval: 'month', + amount: 1000 + }, { + name: 'Yearly', + currency: 'usd', + interval: 'year', + amount: 10000 + }] + } + }] + }; + + const stripeConnectIntegration = { + secret_key: setConnect ? 'connect_secret' : null, + public_key: setConnect ? 'connect_public' : null, + livemode: true + }; + + const getStub = sinon.stub(); + + getStub.withArgs('members_subscription_settings').returns(membersSubscriptionSettings); + getStub.withArgs('stripe_connect_integration').returns(stripeConnectIntegration); + return { + get: getStub + }; +} + +describe('Members - config', function () { + it('Uses direct keys when stripeDirect is true, regardles of which keys exist', function () { + const config = createConfigMock({stripeDirectValue: true}); + const settingsCache = createSettingsMock({setDirect: true, setConnect: Math.random() < 0.5}); + + const membersConfig = new MembersConfigProvider({ + config, + settingsCache, + urlUtils, + ghostVersion: {original: 'v7357'}, + logging: console + }); + + const paymentConfig = membersConfig.getStripePaymentConfig(); + + should.equal(paymentConfig.publicKey, 'direct_public'); + should.equal(paymentConfig.secretKey, 'direct_secret'); + }); + + it('Does not use connect keys if stripeDirect is true, and the direct keys do not exist', function () { + const config = createConfigMock({stripeDirectValue: true}); + const settingsCache = createSettingsMock({setDirect: false, setConnect: true}); + + const membersConfig = new MembersConfigProvider({ + config, + settingsCache, + urlUtils, + ghostVersion: {original: 'v7357'}, + logging: console + }); + + const paymentConfig = membersConfig.getStripePaymentConfig(); + + should.equal(paymentConfig, null); + }); + + it('Uses connect keys when stripeDirect is false, and the connect keys exist', function () { + const config = createConfigMock({stripeDirectValue: false}); + const settingsCache = createSettingsMock({setDirect: true, setConnect: true}); + + const membersConfig = new MembersConfigProvider({ + config, + settingsCache, + urlUtils, + ghostVersion: {original: 'v7357'}, + logging: console + }); + + const paymentConfig = membersConfig.getStripePaymentConfig(); + + should.equal(paymentConfig.publicKey, 'connect_public'); + should.equal(paymentConfig.secretKey, 'connect_secret'); + }); + + it('Uses direct keys when stripeDirect is false, but the connect keys do not exist', function () { + const config = createConfigMock({stripeDirectValue: false}); + const settingsCache = createSettingsMock({setDirect: true, setConnect: false}); + + const membersConfig = new MembersConfigProvider({ + config, + settingsCache, + urlUtils, + ghostVersion: {original: 'v7357'}, + logging: console + }); + + const paymentConfig = membersConfig.getStripePaymentConfig(); + + should.equal(paymentConfig.publicKey, 'direct_public'); + should.equal(paymentConfig.secretKey, 'direct_secret'); + }); +});