From c8b6b8384879cc51e1feee4227d2c4a82d7db130 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Thu, 11 Jul 2019 10:22:25 -0500 Subject: [PATCH] Merge pull request #1903 from overleaf/jel-oauth-email-notification OAuth link/unlink email notification via v2 GitOrigin-RevId: 36b0c6153d3eb8174adc4fd684837d81be95b644 --- .../app/src/Features/Email/EmailBuilder.js | 33 +++++ .../User/ThirdPartyIdentityManager.js | 125 +++++++++++++----- .../acceptance/config/settings.test.coffee | 12 ++ .../src/UserThirdPartyIdentityTests.js | 42 +++--- 4 files changed, 157 insertions(+), 55 deletions(-) diff --git a/services/web/app/src/Features/Email/EmailBuilder.js b/services/web/app/src/Features/Email/EmailBuilder.js index dddc1ac06a..67fa99d895 100644 --- a/services/web/app/src/Features/Email/EmailBuilder.js +++ b/services/web/app/src/Features/Email/EmailBuilder.js @@ -23,6 +23,7 @@ const BaseWithHeaderEmailLayout = require(`./Layouts/${ }BaseWithHeaderEmailLayout`) const SpamSafe = require('./SpamSafe') +// Single CTA Email const SingleCTAEmailBody = require(`./Bodies/${ settings.brandPrefix }SingleCTAEmailBody`) @@ -476,6 +477,38 @@ If you have any questions, you can contact our support team by reply.\ } }) +templates.emailThirdPartyIdentifierLinked = NoCTAEmailTemplate({ + subject(opts) { + return `Your ${settings.appName} account is now linked with ${ + opts.provider + }` + }, + title(opts) { + return `Accounts Linked` + }, + message(opts) { + let message = `We're contacting you to notify you that your ${opts.provider} + account is now linked to your ${settings.appName} account` + return message + } +}) + +templates.emailThirdPartyIdentifierUnlinked = NoCTAEmailTemplate({ + subject(opts) { + return `Your ${settings.appName} account is no longer linked with ${ + opts.provider + }` + }, + title(opts) { + return `Accounts No Longer Linked` + }, + message(opts) { + let message = `We're contacting you to notify you that your ${opts.provider} + account is no longer linked with your ${settings.appName} account.` + return message + } +}) + module.exports = { templates, CTAEmailTemplate, diff --git a/services/web/app/src/Features/User/ThirdPartyIdentityManager.js b/services/web/app/src/Features/User/ThirdPartyIdentityManager.js index 9aa0fdf206..8886688981 100644 --- a/services/web/app/src/Features/User/ThirdPartyIdentityManager.js +++ b/services/web/app/src/Features/User/ThirdPartyIdentityManager.js @@ -1,8 +1,12 @@ +const EmailHandler = require('../../../../app/src/Features/Email/EmailHandler') const Errors = require('../Errors/Errors') +const logger = require('logger-sharelatex') const { User } = require('../../models/User') -const UserUpdater = require('./UserUpdater') +const settings = require('settings-sharelatex') const _ = require('lodash') +const oauthProviders = settings.oauthProviders || {} + const ThirdPartyIdentityManager = (module.exports = { getUser(providerId, externalUserId, callback) { if (providerId == null || externalUserId == null) { @@ -77,6 +81,9 @@ const ThirdPartyIdentityManager = (module.exports = { // is complete link(userId, providerId, externalUserId, externalData, callback, retry) { + if (!oauthProviders[providerId]) { + return callback(new Error('Not a valid provider')) + } const query = { _id: userId, 'thirdPartyIdentifiers.providerId': { @@ -93,38 +100,65 @@ const ThirdPartyIdentityManager = (module.exports = { } } // add new tpi only if an entry for the provider does not exist - UserUpdater.updateUser(query, update, function(err, res) { - if (err && err.code === 11000) { - return callback(new Errors.ThirdPartyIdentityExistsError()) - } - if (err != null) { - return callback(err) - } - if (res.nModified === 1) { - return callback(null, res) - } - // if already retried then throw error - if (retry) { - return callback(new Error('update failed')) - } - // attempt to clear existing entry then retry - ThirdPartyIdentityManager.unlink(userId, providerId, function(err) { - if (err != null) { - return callback(err) + // projection includes thirdPartyIdentifiers for tests + User.findOneAndUpdate( + query, + update, + { projection: { email: 1, thirdPartyIdentifiers: 1 }, new: 1 }, + (err, res) => { + if (err && err.code === 11000) { + callback(new Errors.ThirdPartyIdentityExistsError()) + } else if (err != null) { + callback(err) + } else if (res) { + const emailOptions = { + to: res.email, + provider: oauthProviders[providerId].name + } + if (settings.oauthFallback) { + return callback(null, res) + } else { + EmailHandler.sendEmail( + 'emailThirdPartyIdentifierLinked', + emailOptions, + error => { + if (error != null) { + logger.warn(error) + } + return callback(null, res) + } + ) + } + } else if (retry) { + // if already retried then throw error + callback(new Error('update failed')) + } else { + // attempt to clear existing entry then retry + ThirdPartyIdentityManager.unlink(userId, providerId, function(err) { + if (err != null) { + return callback(err) + } + ThirdPartyIdentityManager.link( + userId, + providerId, + externalUserId, + externalData, + callback, + retry + ) + }) } - ThirdPartyIdentityManager.link( - userId, - providerId, - externalUserId, - externalData, - callback, - true - ) - }) - }) + } + ) }, unlink(userId, providerId, callback) { + if (!oauthProviders[providerId]) { + return callback(new Error('Not a valid provider')) + } + const query = { + _id: userId + } const update = { $pull: { thirdPartyIdentifiers: { @@ -132,6 +166,37 @@ const ThirdPartyIdentityManager = (module.exports = { } } } - UserUpdater.updateUser(userId, update, callback) + // projection includes thirdPartyIdentifiers for tests + User.findOneAndUpdate( + query, + update, + { projection: { email: 1, thirdPartyIdentifiers: 1 }, new: 1 }, + (err, res) => { + if (err != null) { + callback(err) + } else if (!res) { + callback(new Error('update failed')) + } else { + const emailOptions = { + to: res.email, + provider: oauthProviders[providerId].name + } + if (settings.oauthFallback) { + return callback(null, res) + } else { + EmailHandler.sendEmail( + 'emailThirdPartyIdentifierUnlinked', + emailOptions, + error => { + if (error != null) { + logger.warn(error) + } + return callback(null, res) + } + ) + } + } + } + ) } }) diff --git a/services/web/test/acceptance/config/settings.test.coffee b/services/web/test/acceptance/config/settings.test.coffee index 5252f03834..b493d65263 100644 --- a/services/web/test/acceptance/config/settings.test.coffee +++ b/services/web/test/acceptance/config/settings.test.coffee @@ -144,3 +144,15 @@ module.exports = authWithV1: true url: '/docs' } + + oauthProviders: + 'provider': { + name: 'provider' + }, + 'collabratec': { + name: 'collabratec' + } + 'google': { + name: 'google' + }, + diff --git a/services/web/test/acceptance/src/UserThirdPartyIdentityTests.js b/services/web/test/acceptance/src/UserThirdPartyIdentityTests.js index 7e89edce7b..d7be189cbd 100644 --- a/services/web/test/acceptance/src/UserThirdPartyIdentityTests.js +++ b/services/web/test/acceptance/src/UserThirdPartyIdentityTests.js @@ -93,13 +93,13 @@ describe('ThirdPartyIdentityManager', function() { describe('link', function() { describe('when provider not already linked', () => it('should link provider to user', function(done) { - return ThirdPartyIdentityManager.link( + ThirdPartyIdentityManager.link( this.user.id, this.provider, this.externalUserId, this.externalData, function(err, res) { - expect(res.nModified).to.equal(1) + expect(res.thirdPartyIdentifiers.length).to.equal(1) return done() } ) @@ -107,7 +107,7 @@ describe('ThirdPartyIdentityManager', function() { describe('when provider is already linked', function() { beforeEach(function(done) { - return ThirdPartyIdentityManager.link( + ThirdPartyIdentityManager.link( this.user.id, this.provider, this.externalUserId, @@ -117,29 +117,27 @@ describe('ThirdPartyIdentityManager', function() { }) it('should link provider to user', function(done) { - return ThirdPartyIdentityManager.link( + ThirdPartyIdentityManager.link( this.user.id, this.provider, this.externalUserId, this.externalData, function(err, res) { - expect(res.nModified).to.equal(1) - return done() + expect(res).to.exist + done() } ) }) it('should not create duplicate thirdPartyIdentifiers', function(done) { - return ThirdPartyIdentityManager.link( + ThirdPartyIdentityManager.link( this.user.id, this.provider, this.externalUserId, this.externalData, - (err, res) => { - return this.user.get(function(err, user) { - expect(user.thirdPartyIdentifiers.length).to.equal(1) - return done() - }) + function(err, user) { + expect(user.thirdPartyIdentifiers.length).to.equal(1) + return done() } ) }) @@ -151,13 +149,9 @@ describe('ThirdPartyIdentityManager', function() { this.provider, this.externalUserId, this.externalData, - (err, res) => { - return this.user.get((err, user) => { - expect(user.thirdPartyIdentifiers[0].externalData).to.deep.equal( - this.externalData - ) - return done() - }) + (err, user) => { + expect(user.thirdPartyIdentifiers.length).to.equal(1) + return done() } ) }) @@ -193,7 +187,7 @@ describe('ThirdPartyIdentityManager', function() { this.provider, function(err, res) { expect(err).to.be.null - expect(res.nModified).to.equal(0) + expect(res.thirdPartyIdentifiers.length).to.equal(0) return done() } ) @@ -214,11 +208,9 @@ describe('ThirdPartyIdentityManager', function() { return ThirdPartyIdentityManager.unlink( this.user.id, this.provider, - (err, res) => { - return this.user.get(function(err, user) { - expect(user.thirdPartyIdentifiers.length).to.equal(0) - return done() - }) + (err, user) => { + expect(user.thirdPartyIdentifiers.length).to.equal(0) + return done() } ) })