Ver código fonte

Merge pull request #5242 from overleaf/hb-update-project-invite-emails

Update wording for project invite email to account for spam safe cases

GitOrigin-RevId: 80a171548b1c496be2e61b61895ca51cd714f2f7
Timothée Alby 5 anos atrás
pai
commit
1ad94859d5

+ 46 - 19
services/web/app/src/Features/Email/EmailBuilder.js

@@ -105,7 +105,7 @@ ${content.message(opts, true).join('\r\n\r\n')}
 
 Regards,
 The ${settings.appName} Team - ${settings.siteUrl}\
-`
+      `
     },
     compiledTemplate(opts) {
       return NoCTAEmailBody({
@@ -244,27 +244,54 @@ templates.confirmEmail = ctaTemplate({
 
 templates.projectInvite = ctaTemplate({
   subject(opts) {
-    return `${_.escape(
-      SpamSafe.safeProjectName(opts.project.name, 'New Project')
-    )} - shared by ${_.escape(
-      SpamSafe.safeEmail(opts.owner.email, 'a collaborator')
-    )}`
+    const safeName = SpamSafe.isSafeProjectName(opts.project.name)
+    const safeEmail = SpamSafe.isSafeEmail(opts.owner.email)
+
+    if (safeName && safeEmail) {
+      return `"${_.escape(opts.project.name)}" — shared by ${_.escape(
+        opts.owner.email
+      )}`
+    }
+    if (safeName) {
+      return `${settings.appName} project shared with you — "${_.escape(
+        opts.project.name
+      )}"`
+    }
+    if (safeEmail) {
+      return `${_.escape(opts.owner.email)} shared an ${
+        settings.appName
+      } project with you`
+    }
+
+    return `An ${settings.appName} project has been shared with you`
   },
   title(opts) {
-    return `${_.escape(
-      SpamSafe.safeProjectName(opts.project.name, 'New Project')
-    )} - shared by ${_.escape(
-      SpamSafe.safeEmail(opts.owner.email, 'a collaborator')
-    )}`
+    return 'Project Invite'
   },
-  message(opts) {
-    return [
-      `${_.escape(
-        SpamSafe.safeEmail(opts.owner.email, 'a collaborator')
-      )} wants to share ${_.escape(
-        SpamSafe.safeProjectName(opts.project.name, 'a new project')
-      )} with you.`,
-    ]
+  greeting(opts) {
+    return ''
+  },
+  message(opts, isPlainText) {
+    // build message depending on spam-safe variables
+    var message = [`You have been invited to an ${settings.appName} project.`]
+
+    if (SpamSafe.isSafeProjectName(opts.project.name)) {
+      message.push('<br/> Project:')
+      message.push(`<b>${_.escape(opts.project.name)}</b>`)
+    }
+
+    if (SpamSafe.isSafeEmail(opts.owner.email)) {
+      message.push(`<br/> Shared by:`)
+      message.push(`<b>${_.escape(opts.owner.email)}</b>`)
+    }
+
+    if (message.length === 1) {
+      message.push('<br/> Please view the project to find out more.')
+    }
+
+    return message.map(m => {
+      return EmailMessageHelper.cleanHTML(m, isPlainText)
+    })
   },
   ctaText() {
     return 'View project'

+ 38 - 10
services/web/test/unit/src/Email/EmailBuilderTests.js

@@ -63,18 +63,44 @@ describe('EmailBuilder', function () {
     })
 
     describe('when someone is up to no good', function () {
-      beforeEach(function () {
+      it('should not contain the project name at all if unsafe', function () {
         this.opts.project.name = "<img src='http://evilsite.com/evil.php'>"
         this.email = this.EmailBuilder.buildEmail('projectInvite', this.opts)
+        expect(this.email.html).to.not.contain('evilsite.com')
+        expect(this.email.subject).to.not.contain('evilsite.com')
+
+        // but email should appear
+        expect(this.email.html).to.contain(this.opts.owner.email)
+        expect(this.email.subject).to.contain(this.opts.owner.email)
       })
 
-      it('should not contain unescaped html in the html part', function () {
-        expect(this.email.html).to.contain('New Project')
+      it('should not contain the inviter email at all if unsafe', function () {
+        this.opts.owner.email =
+          'verylongemailaddressthatwillfailthecheck@longdomain.domain'
+        this.email = this.EmailBuilder.buildEmail('projectInvite', this.opts)
+
+        expect(this.email.html).to.not.contain(this.opts.owner.email)
+        expect(this.email.subject).to.not.contain(this.opts.owner.email)
+
+        // but title should appear
+        expect(this.email.html).to.contain(this.opts.project.name)
+        expect(this.email.subject).to.contain(this.opts.project.name)
       })
 
-      it('should not have undefined in it', function () {
-        this.email.html.indexOf('undefined').should.equal(-1)
-        this.email.subject.indexOf('undefined').should.equal(-1)
+      it('should handle both email and title being unsafe', function () {
+        this.opts.project.name = "<img src='http://evilsite.com/evil.php'>"
+        this.opts.owner.email =
+          'verylongemailaddressthatwillfailthecheck@longdomain.domain'
+        this.email = this.EmailBuilder.buildEmail('projectInvite', this.opts)
+
+        expect(this.email.html).to.not.contain('evilsite.com')
+        expect(this.email.subject).to.not.contain('evilsite.com')
+        expect(this.email.html).to.not.contain(this.opts.owner.email)
+        expect(this.email.subject).to.not.contain(this.opts.owner.email)
+
+        expect(this.email.html).to.contain(
+          'Please view the project to find out more'
+        )
       })
     })
   })
@@ -84,7 +110,7 @@ describe('EmailBuilder', function () {
       this.opts = {
         to: 'bob@joe.com',
         first_name: 'bob',
-        owner: {
+        newOwner: {
           email: 'sally@hally.com',
         },
         inviteUrl: 'http://example.com/invite',
@@ -93,12 +119,14 @@ describe('EmailBuilder', function () {
           name: 'come buy my product at http://notascam.com',
         },
       }
-      this.email = this.EmailBuilder.buildEmail('projectInvite', this.opts)
+      this.email = this.EmailBuilder.buildEmail(
+        'ownershipTransferConfirmationPreviousOwner',
+        this.opts
+      )
     })
 
     it('should replace spammy project name', function () {
-      this.email.html.indexOf('a new project').should.not.equal(-1)
-      this.email.subject.indexOf('New Project').should.not.equal(-1)
+      this.email.html.indexOf('your project').should.not.equal(-1)
     })
   })