Feat: add auth provider to users on sign in (#3739)

* feat: merge a new authentication method onto existing user records when emails match

* adds test for invite acceptance and auth provider creation

* addresses comments
- test existing user and invites in different test cases
- update lastActiveAt syncronously when an invite is accepted

* sort arrays in test to prevent nondeterministic test behaivior when doing array compare

server/commands/userCreator.test.ts

@@ -1,3 +1,4 @@
+import { v4 as uuidv4 } from "uuid";
 import { TeamDomain } from "@server/models";
 import { buildUser, buildTeam, buildInvite } from "@server/test/factories";
 import { flushdb, seed } from "@server/test/support";
@@ -37,6 +38,77 @@ describe("userCreator", () => {
     expect(isNewUser).toEqual(false);
   });
 
+  it("should add authentication provider to existing users", async () => {
+    const team = await buildTeam({ inviteRequired: true });
+    const teamAuthProviders = await team.$get("authenticationProviders");
+    const authenticationProvider = teamAuthProviders[0];
+
+    const email = "mynam@email.com";
+    const existing = await buildUser({
+      email,
+      teamId: team.id,
+      authentications: [],
+    });
+
+    const result = await userCreator({
+      name: existing.name,
+      email,
+      username: "new-username",
+      avatarUrl: existing.avatarUrl,
+      teamId: existing.teamId,
+      ip,
+      authentication: {
+        authenticationProviderId: authenticationProvider.id,
+        providerId: uuidv4(),
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+    const { user, authentication, isNewUser } = result;
+    expect(authentication.accessToken).toEqual("123");
+    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication.scopes[0]).toEqual("read");
+
+    const authentications = await user.$get("authentications");
+    expect(authentications.length).toEqual(1);
+    expect(isNewUser).toEqual(false);
+  });
+
+  it("should add authentication provider to invited users", async () => {
+    const team = await buildTeam({ inviteRequired: true });
+    const teamAuthProviders = await team.$get("authenticationProviders");
+    const authenticationProvider = teamAuthProviders[0];
+
+    const email = "mynam@email.com";
+    const existing = await buildInvite({
+      email,
+      teamId: team.id,
+    });
+
+    const result = await userCreator({
+      name: existing.name,
+      email,
+      username: "new-username",
+      avatarUrl: existing.avatarUrl,
+      teamId: existing.teamId,
+      ip,
+      authentication: {
+        authenticationProviderId: authenticationProvider.id,
+        providerId: uuidv4(),
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+    const { user, authentication, isNewUser } = result;
+    expect(authentication.accessToken).toEqual("123");
+    expect(authentication.scopes.length).toEqual(1);
+    expect(authentication.scopes[0]).toEqual("read");
+
+    const authentications = await user.$get("authentications");
+    expect(authentications.length).toEqual(1);
+    expect(isNewUser).toEqual(true);
+  });
+
   it("should create user with deleted user matching providerId", async () => {
     const existing = await buildUser();
     const authentications = await existing.$get("authentications");

server/commands/userCreator.ts

@@ -1,4 +1,3 @@
-import { Op } from "sequelize";
 import { sequelize } from "@server/database/sequelize";
 import InviteAcceptedEmail from "@server/emails/templates/InviteAcceptedEmail";
 import { DomainNotAllowedError, InviteRequiredError } from "@server/errors";
@@ -89,48 +88,62 @@ export default async function userCreator({
   // A `user` record might exist in the form of an invite even if there is no
   // existing authentication record that matches. In Outline an invite is a
   // shell user record.
-  const invite = await User.scope(["withAuthentications", "withTeam"]).findOne({
+  const existingUser = await User.scope([
+    "withAuthentications",
+    "withTeam",
+  ]).findOne({
     where: {
       // Email from auth providers may be capitalized and we should respect that
       // however any existing invites will always be lowercased.
       email: email.toLowerCase(),
       teamId,
-      lastActiveAt: {
-        [Op.is]: null,
-      },
     },
   });
 
   // We have an existing invite for his user, so we need to update it with our
   // new details, link up the authentication method, and count this as a new
   // user creation.
-  if (invite && !invite.authentications.length) {
+  if (existingUser) {
+    // A `user` record might exist in the form of an invite.
+    // In Outline an invite is a shell user record with no authentication method
+    // that's never been active before.
+    const isInvite = existingUser.isInvited;
+
     const auth = await sequelize.transaction(async (transaction) => {
-      await invite.update(
+      if (isInvite) {
+        await Event.create(
+          {
+            name: "users.create",
+            actorId: existingUser.id,
+            userId: existingUser.id,
+            teamId: existingUser.teamId,
+            data: {
+              name,
+            },
+            ip,
+          },
+          {
+            transaction,
+          }
+        );
+      }
+
+      // Regardless, create a new authentication record
+      // against the existing user (user can auth with multiple SSO providers)
+      // Update user's name and avatar based on the most recently added provider
+      await existingUser.update(
         {
           name,
           avatarUrl,
+          lastActiveAt: new Date(),
+          lastActiveIp: ip,
         },
         {
           transaction,
         }
       );
-      await Event.create(
+
-        {
+      return await existingUser.$create<UserAuthentication>(
-          name: "users.create",
-          actorId: invite.id,
-          userId: invite.id,
-          teamId: invite.teamId,
-          data: {
-            name,
-          },
-          ip,
-        },
-        {
-          transaction,
-        }
-      );
-      return await invite.$create<UserAuthentication>(
         "authentication",
         authentication,
         {
@@ -139,20 +152,22 @@ export default async function userCreator({
       );
     });
 
-    const inviter = await invite.$get("invitedBy");
+    if (isInvite) {
-    if (inviter) {
+      const inviter = await existingUser.$get("invitedBy");
-      await InviteAcceptedEmail.schedule({
+      if (inviter) {
-        to: inviter.email,
+        await InviteAcceptedEmail.schedule({
-        inviterId: inviter.id,
+          to: inviter.email,
-        invitedName: invite.name,
+          inviterId: inviter.id,
-        teamUrl: invite.team.url,
+          invitedName: existingUser.name,
-      });
+          teamUrl: existingUser.team.url,
+        });
+      }
     }
 
     return {
-      user: invite,
+      user: existingUser,
       authentication: auth,
-      isNewUser: true,
+      isNewUser: isInvite,
     };
   }
 
@@ -165,9 +180,9 @@ export default async function userCreator({
       transaction,
     });
 
-    // If the team settings are set to require invites, and the user is not already invited,
+    // If the team settings are set to require invites, and there's no existing user record,
     // throw an error and fail user creation.
-    if (team?.inviteRequired && !invite) {
+    if (team?.inviteRequired) {
       throw InviteRequiredError();
     }
 

server/routes/api/team.test.ts

@@ -39,7 +39,7 @@ describe("#team.update", () => {
     });
     const body = await res.json();
     expect(res.status).toEqual(200);
-    expect(body.data.allowedDomains).toEqual([
+    expect(body.data.allowedDomains.sort()).toEqual([
       "example-company.com",
       "example-company.org",
     ]);
@@ -47,7 +47,7 @@ describe("#team.update", () => {
     const teamDomains: TeamDomain[] = await TeamDomain.findAll({
       where: { teamId: team.id },
     });
-    expect(teamDomains.map((d) => d.name)).toEqual([
+    expect(teamDomains.map((d) => d.name).sort()).toEqual([
       "example-company.com",
       "example-company.org",
     ]);

server/test/factories.ts

@@ -179,6 +179,7 @@ export async function buildInvite(overrides: Partial<User> = {}) {
     name: `User ${count}`,
     createdAt: new Date("2018-01-01T00:00:00.000Z"),
     invitedById: actor.id,
+    authentications: [],
     ...overrides,
     lastActiveAt: null,
   });