diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index 10d99a4b15..dd464094e6 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -361,14 +361,16 @@ module.exports = SubscriptionController = { ) { const recurlySubscription = req.body['expired_subscription_notification'].subscription - return SubscriptionHandler.recurlyCallback(recurlySubscription, function( - err - ) { - if (err != null) { - return next(err) + return SubscriptionHandler.recurlyCallback( + recurlySubscription, + { ip: req.ip }, + function(err) { + if (err != null) { + return next(err) + } + return res.sendStatus(200) } - return res.sendStatus(200) - }) + ) } else { return res.sendStatus(200) } diff --git a/services/web/app/src/Features/Subscription/SubscriptionHandler.js b/services/web/app/src/Features/Subscription/SubscriptionHandler.js index 9088d66fad..77fbd8985a 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionHandler.js +++ b/services/web/app/src/Features/Subscription/SubscriptionHandler.js @@ -209,7 +209,7 @@ const SubscriptionHandler = { }) }, - recurlyCallback(recurlySubscription, callback) { + recurlyCallback(recurlySubscription, requesterData, callback) { return RecurlyWrapper.getSubscription( recurlySubscription.uuid, { includeAccount: true }, @@ -230,6 +230,7 @@ const SubscriptionHandler = { return SubscriptionUpdater.syncSubscription( recurlySubscription, user != null ? user._id : undefined, + requesterData, callback ) }) diff --git a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js index 3451868d22..8ba381b829 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js +++ b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js @@ -8,6 +8,7 @@ const PlansLocator = require('./PlansLocator') const logger = require('logger-sharelatex') const { ObjectId } = require('mongoose').Types const FeaturesUpdater = require('./FeaturesUpdater') +const { DeletedSubscription } = require('../../models/DeletedSubscription') const SubscriptionUpdater = { /** @@ -33,7 +34,11 @@ const SubscriptionUpdater = { Subscription.update(query, update, callback) }, - syncSubscription(recurlySubscription, adminUserId, callback) { + syncSubscription(recurlySubscription, adminUserId, requesterData, callback) { + if (!callback) { + callback = requesterData + requesterData = {} + } logger.log( { adminUserId, recurlySubscription }, 'syncSubscription, creating new if subscription does not exist' @@ -53,6 +58,7 @@ const SubscriptionUpdater = { SubscriptionUpdater._updateSubscriptionFromRecurly( recurlySubscription, subscription, + requesterData, callback ) } else { @@ -70,6 +76,7 @@ const SubscriptionUpdater = { SubscriptionUpdater._updateSubscriptionFromRecurly( recurlySubscription, subscription, + requesterData, callback ) }) @@ -172,11 +179,11 @@ const SubscriptionUpdater = { Subscription.deleteOne({ 'overleaf.id': v1TeamId }, callback) }, - deleteSubscription(subscriptionId, callback) { + deleteSubscription(subscription, deleterData, callback) { if (callback == null) { callback = function() {} } - SubscriptionLocator.getSubscription(subscriptionId, function( + SubscriptionLocator.getSubscription(subscription._id, function( err, subscription ) { @@ -187,22 +194,46 @@ const SubscriptionUpdater = { subscription.member_ids || [] ) logger.log( - { subscriptionId, affectedUserIds }, + { subscriptionId: subscription._id, affectedUserIds }, 'deleting subscription and downgrading users' ) - Subscription.remove({ _id: ObjectId(subscriptionId) }, function(err) { - if (err != null) { - return callback(err) + SubscriptionUpdater._createDeletedSubscription( + subscription, + deleterData, + error => { + if (error) { + return callback(error) + } + Subscription.remove({ _id: subscription._id }, function(err) { + if (err != null) { + return callback(err) + } + async.mapSeries( + affectedUserIds, + FeaturesUpdater.refreshFeatures, + callback + ) + }) } - async.mapSeries( - affectedUserIds, - FeaturesUpdater.refreshFeatures, - callback - ) - }) + ) }) }, + _createDeletedSubscription(subscription, deleterData, callback) { + subscription.teamInvites = [] + subscription.invited_emails = [] + const filter = { 'subscription._id': subscription._id } + const data = { + deleterData: { + deleterId: deleterData.id, + deleterIpAddress: deleterData.ip + }, + subscription: subscription + } + const options = { upsert: true, new: true, setDefaultsOnInsert: true } + DeletedSubscription.findOneAndUpdate(filter, data, options, callback) + }, + _createNewSubscription(adminUserId, callback) { logger.log({ adminUserId }, 'creating new subscription') const subscription = new Subscription({ @@ -212,10 +243,19 @@ const SubscriptionUpdater = { subscription.save(err => callback(err, subscription)) }, - _updateSubscriptionFromRecurly(recurlySubscription, subscription, callback) { + _updateSubscriptionFromRecurly( + recurlySubscription, + subscription, + requesterData, + callback + ) { logger.log({ recurlySubscription, subscription }, 'updaing subscription') if (recurlySubscription.state === 'expired') { - return SubscriptionUpdater.deleteSubscription(subscription._id, callback) + return SubscriptionUpdater.deleteSubscription( + subscription, + requesterData, + callback + ) } subscription.recurlySubscription_id = recurlySubscription.uuid subscription.planCode = recurlySubscription.plan.plan_code diff --git a/services/web/app/src/models/DeletedSubscription.js b/services/web/app/src/models/DeletedSubscription.js new file mode 100644 index 0000000000..a0c304ad5d --- /dev/null +++ b/services/web/app/src/models/DeletedSubscription.js @@ -0,0 +1,42 @@ +const mongoose = require('mongoose') +const Settings = require('settings-sharelatex') +const { SubscriptionSchema } = require('./Subscription') + +const { Schema } = mongoose +const { ObjectId } = Schema + +const DeleterDataSchema = new Schema( + { + deleterId: { type: ObjectId, ref: 'User' }, + deleterIpAddress: { type: String }, + deletedAt: { + type: Date, + default() { + return new Date() + } + } + }, + { _id: false } +) + +const DeletedSubscriptionSchema = new Schema( + { + deleterData: DeleterDataSchema, + subscription: SubscriptionSchema + }, + { collection: 'deletedSubscriptions' } +) + +const conn = mongoose.createConnection(Settings.mongo.url, { + server: { poolSize: Settings.mongo.poolSize || 10 }, + config: { autoIndex: false } +}) + +const DeletedSubscription = conn.model( + 'DeletedSubscription', + DeletedSubscriptionSchema +) + +mongoose.model('DeletedSubscription', DeletedSubscriptionSchema) +exports.DeletedSubscription = DeletedSubscription +exports.DeletedSubscriptionSchema = DeletedSubscriptionSchema diff --git a/services/web/test/acceptance/src/SubscriptionTests.js b/services/web/test/acceptance/src/SubscriptionDashboardTests.js similarity index 100% rename from services/web/test/acceptance/src/SubscriptionTests.js rename to services/web/test/acceptance/src/SubscriptionDashboardTests.js diff --git a/services/web/test/acceptance/src/SubscriptionDeletionTests.js b/services/web/test/acceptance/src/SubscriptionDeletionTests.js new file mode 100644 index 0000000000..dcb48b287a --- /dev/null +++ b/services/web/test/acceptance/src/SubscriptionDeletionTests.js @@ -0,0 +1,73 @@ +const { expect } = require('chai') +const async = require('async') +const request = require('./helpers/request') +const User = require('./helpers/User') +const RecurlySubscription = require('./helpers/RecurlySubscription') +const SubscriptionUpdater = require('../../../app/src/Features/Subscription/SubscriptionUpdater') +require('./helpers/MockV1Api') + +describe('Subscriptions', function() { + describe('deletion', function() { + beforeEach(function(done) { + this.adminUser = new User() + this.memberUser = new User() + async.series( + [ + cb => this.adminUser.ensureUserExists(cb), + cb => this.memberUser.ensureUserExists(cb), + cb => { + this.recurlySubscription = new RecurlySubscription({ + adminId: this.adminUser._id, + memberIds: [this.memberUser._id], + invitedEmails: ['foo@bar.com'], + teamInvites: [{ email: 'foo@baz.com' }], + groupPlan: true, + state: 'expired' + }) + this.subscription = this.recurlySubscription.subscription + this.recurlySubscription.ensureExists(cb) + } + ], + done + ) + }) + + it('deletes via Recurly callback', function(done) { + let url = '/user/subscription/callback' + let body = this.recurlySubscription.buildCallbackXml() + + request.post({ url, body }, (error, { statusCode }) => { + if (error) { + return done(error) + } + expect(statusCode).to.equal(200) + this.subscription.expectDeleted({ ip: '127.0.0.1' }, done) + }) + }) + + it('allows deletion when deletedSubscription exists', function(done) { + let url = '/user/subscription/callback' + let body = this.recurlySubscription.buildCallbackXml() + + // create fake deletedSubscription + SubscriptionUpdater._createDeletedSubscription( + this.subscription, + {}, + error => { + if (error) { + return done(error) + } + + // try deleting the subscription + request.post({ url, body }, (error, { statusCode }) => { + if (error) { + return done(error) + } + expect(statusCode).to.equal(200) + this.subscription.expectDeleted({ ip: '127.0.0.1' }, done) + }) + } + ) + }) + }) +}) diff --git a/services/web/test/acceptance/src/helpers/MockRecurlyApi.js b/services/web/test/acceptance/src/helpers/MockRecurlyApi.js index 40e752c08c..9a67d13e22 100644 --- a/services/web/test/acceptance/src/helpers/MockRecurlyApi.js +++ b/services/web/test/acceptance/src/helpers/MockRecurlyApi.js @@ -26,6 +26,14 @@ module.exports = MockRecurlyApi = { coupons: {}, + addSubscription(subscription) { + this.subscriptions[subscription.uuid] = subscription + }, + + addAccount(account) { + this.accounts[account.id] = account + }, + run() { app.get('/subscriptions/:id', (req, res, next) => { const subscription = this.subscriptions[req.params.id] diff --git a/services/web/test/acceptance/src/helpers/RecurlySubscription.js b/services/web/test/acceptance/src/helpers/RecurlySubscription.js new file mode 100644 index 0000000000..f0df698a20 --- /dev/null +++ b/services/web/test/acceptance/src/helpers/RecurlySubscription.js @@ -0,0 +1,39 @@ +const { ObjectId } = require('../../../../app/src/infrastructure/mongojs') +const Subscription = require('./Subscription') +const MockRecurlyApi = require('./MockRecurlyApi') +const RecurlyWrapper = require('../../../../app/src/Features/Subscription/RecurlyWrapper') + +class RecurlySubscription { + constructor(options = {}) { + this.subscription = new Subscription(options) + + this.uuid = ObjectId().toString() + this.accountId = this.subscription.admin_id.toString() + this.state = options.state || 'active' + } + + ensureExists(callback) { + this.subscription.ensureExists(error => { + if (error) { + return callback(error) + } + MockRecurlyApi.addSubscription({ + uuid: this.uuid, + account_id: this.accountId, + state: this.state + }) + MockRecurlyApi.addAccount({ id: this.accountId }) + callback() + }) + } + + buildCallbackXml() { + return RecurlyWrapper._buildXml('expired_subscription_notification', { + subscription: { + uuid: this.uuid + } + }) + } +} + +module.exports = RecurlySubscription diff --git a/services/web/test/acceptance/src/helpers/Subscription.js b/services/web/test/acceptance/src/helpers/Subscription.js index 0b55618978..d2b4b82656 100644 --- a/services/web/test/acceptance/src/helpers/Subscription.js +++ b/services/web/test/acceptance/src/helpers/Subscription.js @@ -1,12 +1,19 @@ const { ObjectId } = require('../../../../app/src/infrastructure/mongojs') +const { expect } = require('chai') const SubscriptionModel = require('../../../../app/src/models/Subscription') .Subscription +const DeletedSubscriptionModel = require(`../../../../app/src/models/DeletedSubscription`) + .DeletedSubscription class Subscription { constructor(options = {}) { + this.admin_id = options.adminId || ObjectId() this.overleaf = options.overleaf || {} this.groupPlan = options.groupPlan this.manager_ids = [] + this.member_ids = options.memberIds || [] + this.invited_emails = options.invitedEmails || [] + this.teamInvites = options.teamInvites || [] } ensureExists(callback) { @@ -32,6 +39,36 @@ class Subscription { callback ) } + + expectDeleted(deleterData, callback) { + DeletedSubscriptionModel.find( + { 'subscription._id': this._id }, + (error, deletedSubscriptions) => { + if (error) { + return callback(error) + } + expect(deletedSubscriptions.length).to.equal(1) + + const deletedSubscription = deletedSubscriptions[0] + expect(deletedSubscription.subscription.teamInvites).to.be.empty + expect(deletedSubscription.subscription.invited_emails).to.be.empty + expect(deletedSubscription.deleterData.deleterIpAddress).to.equal( + deleterData.ip + ) + if (deleterData.id) { + expect(deletedSubscription.deleterData.deleterId.toString()).to.equal( + deleterData.id.toString() + ) + } else { + expect(deletedSubscription.deleterData.deleterId).to.be.undefined + } + SubscriptionModel.findById(this._id, (error, subscription) => { + expect(subscription).to.be.null + callback(error) + }) + } + ) + } } module.exports = Subscription diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js index 9ccbbb66a8..44898c7fad 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js @@ -60,7 +60,7 @@ describe('SubscriptionController', function() { updateSubscription: sinon.stub().callsArgWith(3), reactivateSubscription: sinon.stub().callsArgWith(1), cancelSubscription: sinon.stub().callsArgWith(1), - recurlyCallback: sinon.stub().callsArgWith(1), + recurlyCallback: sinon.stub().yields(), startFreeTrial: sinon.stub() } diff --git a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js index d038d2a48b..986a2e3660 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js @@ -76,7 +76,7 @@ describe('SubscriptionHandler', function() { this.DropboxHandler = { unlinkAccount: sinon.stub().callsArgWith(1) } this.SubscriptionUpdater = { - syncSubscription: sinon.stub().callsArgWith(2), + syncSubscription: sinon.stub().yields(), startFreeTrial: sinon.stub().callsArgWith(1) } @@ -391,6 +391,7 @@ describe('SubscriptionHandler', function() { } return this.SubscriptionHandler.recurlyCallback( this.activeRecurlySubscription, + {}, done ) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js index 485565d249..5494c66af0 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js @@ -94,6 +94,9 @@ describe('SubscriptionUpdater', function() { this.FeaturesUpdater = { refreshFeatures: sinon.stub().yields() } + this.DeletedSubscription = { + findOneAndUpdate: sinon.stub().yields() + } this.SubscriptionUpdater = SandboxedModule.require(modulePath, { globals: { console: console @@ -111,7 +114,10 @@ describe('SubscriptionUpdater', function() { warn() {} }, 'settings-sharelatex': this.Settings, - './FeaturesUpdater': this.FeaturesUpdater + './FeaturesUpdater': this.FeaturesUpdater, + '../../models/DeletedSubscription': { + DeletedSubscription: this.DeletedSubscription + } } }) }) @@ -153,7 +159,7 @@ describe('SubscriptionUpdater', function() { ) this.SubscriptionUpdater._updateSubscriptionFromRecurly = sinon .stub() - .callsArgWith(2) + .yields() }) it('should update the subscription if the user already is admin of one', function(done) { @@ -214,6 +220,7 @@ describe('SubscriptionUpdater', function() { this.SubscriptionUpdater._updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, + {}, err => { if (err != null) { return done(err) @@ -238,12 +245,13 @@ describe('SubscriptionUpdater', function() { this.SubscriptionUpdater._updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, + {}, err => { if (err != null) { return done(err) } this.SubscriptionUpdater.deleteSubscription - .calledWith(this.subscription._id) + .calledWithMatch(this.subscription) .should.equal(true) done() } @@ -254,6 +262,7 @@ describe('SubscriptionUpdater', function() { this.SubscriptionUpdater._updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, + {}, err => { if (err != null) { return done(err) @@ -282,6 +291,7 @@ describe('SubscriptionUpdater', function() { this.SubscriptionUpdater._updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, + {}, err => { if (err != null) { return done(err) @@ -297,6 +307,7 @@ describe('SubscriptionUpdater', function() { this.SubscriptionUpdater._updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, + {}, err => { if (err != null) { return done(err) @@ -441,8 +452,8 @@ describe('SubscriptionUpdater', function() { describe('deleteSubscription', function() { beforeEach(function(done) { - this.subscription_id = ObjectId().toString() this.subscription = { + _id: ObjectId().toString(), mock: 'subscription', admin_id: ObjectId(), member_ids: [ObjectId(), ObjectId(), ObjectId()] @@ -451,18 +462,18 @@ describe('SubscriptionUpdater', function() { .stub() .yields(null, this.subscription) this.FeaturesUpdater.refreshFeatures = sinon.stub().yields() - this.SubscriptionUpdater.deleteSubscription(this.subscription_id, done) + this.SubscriptionUpdater.deleteSubscription(this.subscription, {}, done) }) it('should look up the subscription', function() { this.SubscriptionLocator.getSubscription - .calledWith(this.subscription_id) + .calledWith(this.subscription._id) .should.equal(true) }) it('should remove the subscription', function() { this.SubscriptionModel.remove - .calledWith({ _id: ObjectId(this.subscription_id) }) + .calledWith({ _id: this.subscription._id }) .should.equal(true) })