fix: Welcome emails should not be sent when inviting a user (#2132)

* chore: Bump nodemailer

* fix: Welcome email sent to invites

* test: Add regression test for emails from accountProvisioner

package.json

@@ -56,7 +56,7 @@
     ]
   },
   "engines": {
-    "node": ">= 12 <=15"
+    "node": ">= 12 <=16"
   },
   "repository": {
     "type": "git",

server/auth/providers/email.js

@@ -3,7 +3,7 @@ import subMinutes from "date-fns/sub_minutes";
 import Router from "koa-router";
 import { find } from "lodash";
 import { AuthorizationError } from "../../errors";
-import mailer from "../../mailer";
+import mailer, { sendEmail } from "../../mailer";
 import methodOverride from "../../middlewares/methodOverride";
 import validation from "../../middlewares/validation";
 import { User, Team } from "../../models";
@@ -97,6 +97,9 @@ router.get("email.callback", async (ctx) => {
     if (user.isSuspended) {
       return ctx.redirect("/?notice=suspended");
     }
+    if (user.isInvited) {
+      sendEmail("welcome", user.email, { teamUrl: user.team.url });
+    }
 
     await user.update({ lastActiveAt: new Date() });
 

server/commands/accountProvisioner.js

@@ -6,6 +6,7 @@ import {
   EmailAuthenticationRequiredError,
   AuthenticationProviderDisabledError,
 } from "../errors";
+import { sendEmail } from "../mailer";
 import { Team, User } from "../models";
 import teamCreator from "./teamCreator";
 import userCreator from "./userCreator";
@@ -85,6 +86,10 @@ export default async function accountProvisioner({
 
     const { isNewUser, user } = result;
 
+    if (isNewUser) {
+      sendEmail("welcome", user.email, { teamUrl: team.url });
+    }
+
     if (isNewTeam) {
       await team.provisionFirstCollection(user.id);
     }

server/commands/accountProvisioner.test.js

@@ -1,9 +1,12 @@
 // @flow
+import { sendEmail } from "../mailer";
 import { Collection, UserAuthentication } from "../models";
 import { buildUser, buildTeam } from "../test/factories";
 import { flushdb } from "../test/support";
 import accountProvisioner from "./accountProvisioner";
 
+jest.mock("../mailer");
+
 jest.mock("aws-sdk", () => {
   const mS3 = { putObject: jest.fn().mockReturnThis(), promise: jest.fn() };
   return {
@@ -12,7 +15,12 @@ jest.mock("aws-sdk", () => {
   };
 });
 
-beforeEach(() => flushdb());
+beforeEach(() => {
+  flushdb();
+
+  // $FlowFixMe
+  sendEmail.mockReset();
+});
 
 describe("accountProvisioner", () => {
   const ip = "127.0.0.1";
@@ -51,6 +59,7 @@ describe("accountProvisioner", () => {
     expect(user.email).toEqual("jenny@example.com");
     expect(isNewUser).toEqual(true);
     expect(isNewTeam).toEqual(true);
+    expect(sendEmail).toHaveBeenCalled();
 
     const collectionCount = await Collection.count();
     expect(collectionCount).toEqual(1);
@@ -65,7 +74,7 @@ describe("accountProvisioner", () => {
     const authentication = authentications[0];
     const newEmail = "test@example.com";
 
-    const { user } = await accountProvisioner({
+    const { user, isNewUser, isNewTeam } = await accountProvisioner({
       ip,
       user: {
         name: existing.name,
@@ -93,6 +102,9 @@ describe("accountProvisioner", () => {
     expect(auth.scopes.length).toEqual(1);
     expect(auth.scopes[0]).toEqual("read");
     expect(user.email).toEqual(newEmail);
+    expect(isNewTeam).toEqual(false);
+    expect(isNewUser).toEqual(false);
+    expect(sendEmail).not.toHaveBeenCalled();
 
     const collectionCount = await Collection.count();
     expect(collectionCount).toEqual(0);
@@ -175,6 +187,7 @@ describe("accountProvisioner", () => {
     expect(auth.scopes[0]).toEqual("read");
     expect(user.email).toEqual("jenny@example.com");
     expect(isNewUser).toEqual(true);
+    expect(sendEmail).toHaveBeenCalled();
 
     const collectionCount = await Collection.count();
     expect(collectionCount).toEqual(0);

server/commands/userCreator.js

@@ -117,7 +117,7 @@ export default async function userCreator({
       throw err;
     }
 
-    return { user: invite, authentication: auth, isNewUser: false };
+    return { user: invite, authentication: auth, isNewUser: true };
   }
 
   // No auth, no user – this is an entirely new sign in.

server/commands/userCreator.test.js

@@ -145,6 +145,6 @@ describe("userCreator", () => {
     expect(authentication.scopes.length).toEqual(1);
     expect(authentication.scopes[0]).toEqual("read");
     expect(user.email).toEqual(invite.email);
-    expect(isNewUser).toEqual(false);
+    expect(isNewUser).toEqual(true);
   });
 });

server/models/User.js

@@ -6,14 +6,12 @@ import JWT from "jsonwebtoken";
 import { v4 as uuidv4 } from "uuid";
 import { languages } from "../../shared/i18n";
 import { ValidationError } from "../errors";
-import { sendEmail } from "../mailer";
 import { DataTypes, sequelize, encryptedFields, Op } from "../sequelize";
 import { DEFAULT_AVATAR_HOST } from "../utils/avatars";
 import { publicS3Endpoint, uploadToS3FromUrl } from "../utils/s3";
 import {
   UserAuthentication,
   Star,
-  Team,
   Collection,
   NotificationSetting,
   ApiKey,
@@ -236,10 +234,6 @@ const removeIdentifyingInfo = async (model, options) => {
 User.beforeDestroy(removeIdentifyingInfo);
 User.beforeSave(uploadAvatar);
 User.beforeCreate(setRandomJwtSecret);
-User.afterCreate(async (user) => {
-  const team = await Team.findByPk(user.teamId);
-  sendEmail("welcome", user.email, { teamUrl: team.url });
-});
 
 // By default when a user signs up we subscribe them to email notifications
 // when documents they created are edited by other team members and onboarding