diff --git a/services/web/app/coffee/Features/Errors/Errors.coffee b/services/web/app/coffee/Features/Errors/Errors.coffee index 061e7f4b0f..fb1d305f7d 100644 --- a/services/web/app/coffee/Features/Errors/Errors.coffee +++ b/services/web/app/coffee/Features/Errors/Errors.coffee @@ -118,6 +118,14 @@ ThirdPartyUserNotFoundError = (message) -> return error ThirdPartyUserNotFoundError.prototype.__proto__ = Error.prototype +SubscriptionAdminDeletionError = (message) -> + message = "subscription admins cannot be deleted" unless message? + error = new Error(message) + error.name = "SubscriptionAdminDeletionError" + error.__proto__ = SubscriptionAdminDeletionError.prototype + return error +SubscriptionAdminDeletionError.prototype.__proto__ = Error.prototype + module.exports = Errors = NotFoundError: NotFoundError ForbiddenError: ForbiddenError @@ -136,3 +144,4 @@ module.exports = Errors = NotInV2Error: NotInV2Error SLInV2Error: SLInV2Error ThirdPartyUserNotFoundError: ThirdPartyUserNotFoundError + SubscriptionAdminDeletionError: SubscriptionAdminDeletionError diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index 877caaec45..ab78878e4e 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -39,8 +39,11 @@ module.exports = UserController = return res.sendStatus(403) deleteMethod user_id, (err) -> if err? - logger.err {user_id}, "error while deleting user account" - return next(err) + if err instanceof Errors.SubscriptionAdminDeletionError + return res.status(422).json(error: err.name) + else + logger.err {user_id}, "error while deleting user account" + return next(err) sessionId = req.sessionID req.logout?() req.session.destroy (err) -> diff --git a/services/web/app/coffee/Features/User/UserDeleter.coffee b/services/web/app/coffee/Features/User/UserDeleter.coffee index 4a448f5880..1a604b5e54 100644 --- a/services/web/app/coffee/Features/User/UserDeleter.coffee +++ b/services/web/app/coffee/Features/User/UserDeleter.coffee @@ -4,6 +4,7 @@ ProjectDeleter = require("../Project/ProjectDeleter") logger = require("logger-sharelatex") SubscriptionHandler = require("../Subscription/SubscriptionHandler") SubscriptionUpdater = require("../Subscription/SubscriptionUpdater") +SubscriptionLocator = require("../Subscription/SubscriptionLocator") UserMembershipsHandler = require("../UserMembership/UserMembershipsHandler") async = require("async") InstitutionsAPI = require("../Institutions/InstitutionsAPI") @@ -20,6 +21,8 @@ module.exports = UserDeleter = return callback(err) if err? return callback(new Errors.NotFoundError("user not found")) unless user? async.series([ + (cb) -> + UserDeleter._ensureCanDeleteUser user, cb (cb) -> UserDeleter._cleanupUser user, cb (cb) -> @@ -40,6 +43,8 @@ module.exports = UserDeleter = return callback(err) logger.log user:user, "deleting user" async.series [ + (cb) -> + UserDeleter._ensureCanDeleteUser user, cb (cb)-> UserDeleter._cleanupUser user, cb (cb)-> @@ -67,3 +72,9 @@ module.exports = UserDeleter = (cb)-> UserMembershipsHandler.removeUserFromAllEntities user._id, cb ], callback) + + _ensureCanDeleteUser: (user, callback) -> + SubscriptionLocator.getUsersSubscription user, (error, subscription) -> + if subscription? + error ||= new Errors.SubscriptionAdminDeletionError() + callback(error) diff --git a/services/web/app/views/user/settings.pug b/services/web/app/views/user/settings.pug index eed72bd92e..12af2160cc 100644 --- a/services/web/app/views/user/settings.pug +++ b/services/web/app/views/user/settings.pug @@ -266,13 +266,15 @@ block content label(style="display: inline")  I understand this will delete all projects in my Overleaf v2 account (and ShareLaTeX account, if any) with email address #[em {{ userDefaultEmail }}] div(ng-if="state.error") - div.alert.alert-danger - | #{translate('generic_something_went_wrong')} - div(ng-if="state.invalidCredentials") - div.alert.alert-danger - | #{translate('email_or_password_wrong_try_again')} + div.alert.alert-danger(ng-switch="state.error.code") + span(ng-switch-when="InvalidCredentialsError") + | #{translate('email_or_password_wrong_try_again')} + span(ng-switch-when="SubscriptionAdminDeletionError") + | #{translate('subscription_admins_cannot_be_deleted')} + span(ng-switch-default) + | #{translate('generic_something_went_wrong')} if settings.createV1AccountOnLogin && settings.overleaf - div(ng-if="state.error || state.invalidCredentials") + div(ng-if="state.error && state.error.code == 'InvalidCredentialsError'") div.alert.alert-info | If you can't remember your password, or if you are using Single-Sign-On with another provider | to sign in (such as Twitter or Google), please diff --git a/services/web/public/src/main/account-settings.js b/services/web/public/src/main/account-settings.js index b9eda22730..c090f12df6 100644 --- a/services/web/public/src/main/account-settings.js +++ b/services/web/public/src/main/account-settings.js @@ -86,8 +86,7 @@ define(['base'], function(App) { confirmV1Purge: false, confirmSharelatexDelete: false, inflight: false, - error: false, - invalidCredentials: false + error: null } $scope.userDefaultEmail = userDefaultEmail @@ -107,8 +106,7 @@ define(['base'], function(App) { $scope.delete = function() { $scope.state.inflight = true - $scope.state.error = false - $scope.state.invalidCredentials = false + $scope.state.error = null return $http({ method: 'POST', url: '/user/delete', @@ -124,17 +122,16 @@ define(['base'], function(App) { .then(function() { $modalInstance.close() $scope.state.inflight = false - $scope.state.error = false - $scope.state.invalidCredentials = false + $scope.state.error = null return setTimeout(() => (window.location = '/login'), 1000) }) .catch(function(response) { const { data, status } = response $scope.state.inflight = false if (status === 403) { - return ($scope.state.invalidCredentials = true) + $scope.state.error = { code: 'InvalidCredentialsError' } } else { - return ($scope.state.error = true) + $scope.state.error = { code: data.error } } }) } diff --git a/services/web/test/unit/coffee/User/UserControllerTests.coffee b/services/web/test/unit/coffee/User/UserControllerTests.coffee index d17d079387..a9f41bfd24 100644 --- a/services/web/test/unit/coffee/User/UserControllerTests.coffee +++ b/services/web/test/unit/coffee/User/UserControllerTests.coffee @@ -167,6 +167,20 @@ describe "UserController", -> done() @UserController.tryDeleteUser @req, @res, @next + describe 'when deleteUser produces a known error', -> + + beforeEach -> + @UserDeleter.deleteUser = sinon.stub().yields( + new Errors.SubscriptionAdminDeletionError() + ) + + it 'should return a json error', (done) -> + @UserController.tryDeleteUser @req, status: (status) -> + expect(status).to.equal 422 + json: (json) -> + expect(json.error).to.equal Errors.SubscriptionAdminDeletionError.name + done() + describe 'when session.destroy produces an error', -> beforeEach -> diff --git a/services/web/test/unit/coffee/User/UserDeleterTests.coffee b/services/web/test/unit/coffee/User/UserDeleterTests.coffee index dc97e0b684..3d1f774e6d 100644 --- a/services/web/test/unit/coffee/User/UserDeleterTests.coffee +++ b/services/web/test/unit/coffee/User/UserDeleterTests.coffee @@ -1,8 +1,10 @@ sinon = require('sinon') chai = require('chai') should = chai.should() +expect = chai.expect modulePath = "../../../../app/js/Features/User/UserDeleter.js" SandboxedModule = require('sandboxed-module') +Errors = require('../../../../app/js/Features/Errors/Errors') describe "UserDeleter", -> @@ -27,6 +29,9 @@ describe "UserDeleter", -> @SubscriptionUpdater = removeUserFromAllGroups: sinon.stub().callsArgWith(1) + @SubscriptionLocator = + getUsersSubscription: sinon.stub().yields(null, null) + @UserMembershipsHandler = removeUserFromAllEntities: sinon.stub().callsArgWith(1) @@ -44,14 +49,18 @@ describe "UserDeleter", -> "../Newsletter/NewsletterManager": @NewsletterManager "../Subscription/SubscriptionHandler": @SubscriptionHandler "../Subscription/SubscriptionUpdater": @SubscriptionUpdater + "../Subscription/SubscriptionLocator": @SubscriptionLocator "../UserMembership/UserMembershipsHandler": @UserMembershipsHandler "../Project/ProjectDeleter": @ProjectDeleter "../Institutions/InstitutionsAPI": deleteAffiliations: @deleteAffiliations "../../infrastructure/mongojs": @mongojs "logger-sharelatex": @logger = { log: sinon.stub(), err: sinon.stub() } + "../Errors/Errors": Errors describe "softDeleteUserForMigration", -> + beforeEach -> + @UserDeleter._ensureCanDeleteUser = sinon.stub().yields(null) it "should delete the user in mongo", (done)-> @UserDeleter.softDeleteUserForMigration @user._id, (err)=> @@ -94,7 +103,19 @@ describe "UserDeleter", -> @UserMembershipsHandler.removeUserFromAllEntities.calledWith(@user._id).should.equal true done() + it "ensures user can be deleted first", (done)-> + @UserDeleter._ensureCanDeleteUser.yields( + new Errors.SubscriptionAdminDeletionError() + ) + @UserDeleter.softDeleteUserForMigration @user._id, (error) => + sinon.assert.calledWith(@UserDeleter._ensureCanDeleteUser, @user) + sinon.assert.notCalled(@user.remove) + expect(error).to.be.instanceof Errors.SubscriptionAdminDeletionError + done() + describe "deleteUser", -> + beforeEach -> + @UserDeleter._ensureCanDeleteUser = sinon.stub().yields(null) it "should delete the user in mongo", (done)-> @UserDeleter.deleteUser @user._id, (err)=> @@ -132,6 +153,16 @@ describe "UserDeleter", -> @UserMembershipsHandler.removeUserFromAllEntities.calledWith(@user._id).should.equal true done() + it "ensures user can be deleted first", (done)-> + @UserDeleter._ensureCanDeleteUser.yields( + new Errors.SubscriptionAdminDeletionError() + ) + @UserDeleter.deleteUser @user._id, (error) => + sinon.assert.calledWith(@UserDeleter._ensureCanDeleteUser, @user) + sinon.assert.notCalled(@user.remove) + expect(error).to.be.instanceof Errors.SubscriptionAdminDeletionError + done() + describe "when unsubscribing from mailchimp fails", -> beforeEach -> @NewsletterManager.unsubscribe = sinon.stub().callsArgWith(1, new Error("something went wrong")) @@ -152,3 +183,22 @@ describe "UserDeleter", -> @UserDeleter.deleteUser @user._id, (err)=> sinon.assert.called(@logger.err) done() + + describe '_ensureCanDeleteUser', -> + it 'should not return error when user can be deleted', (done) -> + @SubscriptionLocator.getUsersSubscription.yields(null, null) + @UserDeleter._ensureCanDeleteUser @user, (error) -> + expect(error).to.not.exist + done() + + it 'should return custom error when user is group admin', (done) -> + @SubscriptionLocator.getUsersSubscription.yields(null, { _id: '123abc' }) + @UserDeleter._ensureCanDeleteUser @user, (error) -> + expect(error).to.be.instanceof Errors.SubscriptionAdminDeletionError + done() + + it 'propagate errors', (done) -> + @SubscriptionLocator.getUsersSubscription.yields(new Error('Some error')) + @UserDeleter._ensureCanDeleteUser @user, (error) -> + expect(error).to.be.instanceof Error + done()