From d56d4c3b4a2aee61e480b7bfc6c8aba6225ba4c2 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 8 Jun 2021 15:03:39 +0200 Subject: [PATCH] Add HTTP Basic Auth to Recurly webhook endpoint (#4054) * Add HTTP authentication to Recurly webhook endpoint Co-authored-by: Eric Mc Sween GitOrigin-RevId: 81c32459d643895c096bc195ae6aef53248418da --- .../Subscription/SubscriptionRouter.js | 3 ++ services/web/config/settings.defaults.js | 2 + .../test/acceptance/config/settings.test.js | 2 + .../src/SubscriptionDeletionTests.js | 39 +++++++++++++++---- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/services/web/app/src/Features/Subscription/SubscriptionRouter.js b/services/web/app/src/Features/Subscription/SubscriptionRouter.js index 16244cc2b7..b9c080262a 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionRouter.js +++ b/services/web/app/src/Features/Subscription/SubscriptionRouter.js @@ -73,6 +73,9 @@ module.exports = { // recurly callback publicApiRouter.post( '/user/subscription/callback', + AuthenticationController.requireBasicAuth({ + [Settings.apis.recurly.webhookUser]: Settings.apis.recurly.webhookPass, + }), SubscriptionController.recurlyNotificationParser, SubscriptionController.recurlyCallback ) diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index 558596e5c7..2955203e76 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -256,6 +256,8 @@ module.exports = { apiVersion: process.env.RECURLY_API_VERSION, subdomain: process.env.RECURLY_SUBDOMAIN || '', publicKey: process.env.RECURLY_PUBLIC_KEY || '', + webhookUser: process.env.RECURLY_WEBHOOK_USER, + webhookPass: process.env.RECURLY_WEBHOOK_PASS, }, geoIpLookup: { url: `http://${ diff --git a/services/web/test/acceptance/config/settings.test.js b/services/web/test/acceptance/config/settings.test.js index eaf0496123..c709a0da80 100644 --- a/services/web/test/acceptance/config/settings.test.js +++ b/services/web/test/acceptance/config/settings.test.js @@ -32,6 +32,8 @@ module.exports = { url: 'http://localhost:6034', subdomain: 'test', apiKey: 'private-nonsense', + webhookUser: 'recurly', + webhookPass: 'webhook', }, }, diff --git a/services/web/test/acceptance/src/SubscriptionDeletionTests.js b/services/web/test/acceptance/src/SubscriptionDeletionTests.js index 2c485167b5..ecccd8b7ee 100644 --- a/services/web/test/acceptance/src/SubscriptionDeletionTests.js +++ b/services/web/test/acceptance/src/SubscriptionDeletionTests.js @@ -4,12 +4,19 @@ const request = require('./helpers/request') const User = require('./helpers/User') const RecurlySubscription = require('./helpers/RecurlySubscription') const SubscriptionUpdater = require('../../../app/src/Features/Subscription/SubscriptionUpdater') +const Settings = require('settings-sharelatex') describe('Subscriptions', function () { describe('deletion', function () { beforeEach(function (done) { this.adminUser = new User() this.memberUser = new User() + this.auth = { + user: Settings.apis.recurly.webhookUser, + pass: Settings.apis.recurly.webhookPass, + sendImmediately: true, + } + async.series( [ cb => this.adminUser.ensureUserExists(cb), @@ -33,11 +40,24 @@ describe('Subscriptions', function () { ) }) - it('deletes via Recurly callback', function (done) { + it('should not allow unauthorized access to the Recurly callback', function (done) { const url = '/user/subscription/callback' const body = this.recurlySubscription.buildCallbackXml() request.post({ url, body }, (error, { statusCode }) => { + if (error) { + return done(error) + } + expect(statusCode).to.equal(401) + done() + }) + }) + + it('deletes via Recurly callback', function (done) { + const url = '/user/subscription/callback' + const body = this.recurlySubscription.buildCallbackXml() + + request.post({ url, body, auth: this.auth }, (error, { statusCode }) => { if (error) { return done(error) } @@ -50,7 +70,7 @@ describe('Subscriptions', function () { const url = '/user/subscription/callback' const body = this.recurlySubscription.buildCallbackXml() - request.post({ url, body }, (error, { statusCode }) => { + request.post({ url, body, auth: this.auth }, (error, { statusCode }) => { if (error) { return done(error) } @@ -75,13 +95,16 @@ describe('Subscriptions', function () { } // try deleting the subscription - request.post({ url, body }, (error, { statusCode }) => { - if (error) { - return done(error) + request.post( + { url, body, auth: this.auth }, + (error, { statusCode }) => { + if (error) { + return done(error) + } + expect(statusCode).to.equal(200) + this.subscription.expectDeleted({ ip: '127.0.0.1' }, done) } - expect(statusCode).to.equal(200) - this.subscription.expectDeleted({ ip: '127.0.0.1' }, done) - }) + ) } ) })