Merge pull request #1903 from overleaf/jel-oauth-email-notification

OAuth link/unlink email notification via v2

GitOrigin-RevId: 36b0c6153d3eb8174adc4fd684837d81be95b644

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,

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)
+              }
+            )
+          }
+        }
+      }
+    )
   }
 })

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'
+		},
+

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()
           }
         )
       })