[next] Auth (#2257)

* feat: improved auth features + performance

* fix: auth check logic

* fix: tests

.dockerignore

@@ -1,30 +1,30 @@
-# excluded because we'll likely need to rebuild this.
-node_modules
-
-# static assets are rebuild in the docker container.
-dist
-
 # tests are not run in the docker container.
 __tests__
+coverage
 
 # we won't use the .git folder in production.
 .git
 
-# hide the environment config.
-.env
+# don't include the dependancies
+node_modules
 
-# don't include logs.
+# don't include any logs
 npm-debug.log*
-yarn-error.log
 
-# hide OS specific files.
+# don't include any yarn files
+yarn-error.log
+yarn.lock
+
+# don't include any OS/editor files
+.env
 .idea/
 .vs
 .docz
 *.swp
 *.DS_STORE
 
-# hide generated files.
+# don't include any generated files
+dist
 *.css.d.ts
 __generated__
-
+README.md.orig

.gitignore

@@ -1,17 +1,23 @@
+# don't include the dependancies
 node_modules
-dist
-.env
+
+# don't include any logs
 npm-debug.log*
+
+# don't include any yarn files
 yarn-error.log
 yarn.lock
-coverage
 
+# don't include any OS/editor files
+.env
 .idea/
 .vs
 .docz
 *.swp
 *.DS_STORE
 
+# don't include any generated files
+dist
 *.css.d.ts
 __generated__
 README.md.orig

src/core/client/admin/containers/AuthCheckContainer.tsx

@@ -52,10 +52,7 @@ class AuthCheckContainer extends React.Component<Props> {
   }
 
   private hasAccess(props: Props = this.props) {
-    const {
-      viewer,
-      settings: { auth },
-    } = props.data!;
+    const { viewer } = props.data!;
     if (viewer) {
       if (
         viewer.role === GQLUSER_ROLE.COMMENTER ||
@@ -66,15 +63,6 @@ class AuthCheckContainer extends React.Component<Props> {
           !can(viewer, props.data.route.data))
       ) {
         return false;
-      } else if (
-        !viewer.email ||
-        !viewer.username ||
-        (!viewer.profiles.some(p => p.__typename === "LocalProfile") &&
-          auth.integrations.local.enabled &&
-          (auth.integrations.local.targetFilter.admin ||
-            auth.integrations.local.targetFilter.stream))
-      ) {
-        return false;
       }
       return true;
     }

src/core/client/admin/containers/RedirectLoginContainer.tsx

@@ -1,103 +0,0 @@
-import { Match, Router, withRouter } from "found";
-import React from "react";
-
-import { RedirectLoginContainerQueryResponse } from "talk-admin/__generated__/RedirectLoginContainerQuery.graphql";
-import {
-  SetRedirectPathMutation,
-  withSetRedirectPathMutation,
-} from "talk-admin/mutations";
-import { graphql } from "talk-framework/lib/relay";
-import { withRouteConfig } from "talk-framework/lib/router";
-import { GQLUSER_ROLE } from "talk-framework/schema";
-
-interface Props {
-  match: Match;
-  router: Router;
-  setRedirectPath: SetRedirectPathMutation;
-  data: RedirectLoginContainerQueryResponse | null;
-}
-
-class RedirectLoginContainer extends React.Component<Props> {
-  constructor(props: Props) {
-    super(props);
-    this.redirectIfNotLoggedIn();
-  }
-
-  public componentWillReceiveProps(nextProps: Props) {
-    this.redirectIfNotLoggedIn(nextProps);
-  }
-
-  private shouldRedirectTo(props: Props = this.props): string | null {
-    if (!props.data) {
-      return null;
-    }
-    const {
-      viewer,
-      settings: { auth },
-    } = props.data!;
-    if (viewer) {
-      if (viewer.role === GQLUSER_ROLE.COMMENTER) {
-        return "/admin/login";
-      } else if (
-        !viewer.email ||
-        !viewer.username ||
-        (!viewer.profiles.some(p => p.__typename === "LocalProfile") &&
-          auth.integrations.local.enabled &&
-          (auth.integrations.local.targetFilter.admin ||
-            auth.integrations.local.targetFilter.stream))
-      ) {
-        return "/admin/login";
-      }
-      return "";
-    }
-    return "/admin/login";
-  }
-
-  private redirectIfNotLoggedIn(props: Props = this.props) {
-    const redirect = this.shouldRedirectTo(props);
-    if (redirect) {
-      const location = props.match.location;
-      props.setRedirectPath({
-        path: location.pathname + location.search + location.hash,
-      });
-      props.router.replace(redirect);
-    }
-  }
-
-  public render() {
-    if (this.shouldRedirectTo()) {
-      return null;
-    }
-    return this.props.children;
-  }
-}
-
-const enhanced = withRouteConfig({
-  query: graphql`
-    query RedirectLoginContainerQuery {
-      viewer {
-        username
-        email
-        profiles {
-          __typename
-        }
-        role
-      }
-      settings {
-        auth {
-          integrations {
-            local {
-              enabled
-              targetFilter {
-                admin
-                stream
-              }
-            }
-          }
-        }
-      }
-    }
-  `,
-})(withRouter(withSetRedirectPathMutation(RedirectLoginContainer)));
-
-export default enhanced;

src/core/client/admin/test/auth/restricted.spec.tsx

@@ -59,21 +59,6 @@ it("show restricted screen for commenters and staff", async () => {
   }
 });
 
-it("show restricted screen when email is not set", async () => {
-  const { testRenderer } = createTestRenderer({ email: "" });
-  await waitForElement(() => within(testRenderer.root).getByTestID("authBox"));
-});
-
-it("show restricted screen when username is not set", async () => {
-  const { testRenderer } = createTestRenderer({ username: "" });
-  await waitForElement(() => within(testRenderer.root).getByTestID("authBox"));
-});
-
-it("show restricted screen local was not set (password)", async () => {
-  const { testRenderer } = createTestRenderer({ profiles: [] });
-  await waitForElement(() => within(testRenderer.root).getByTestID("authBox"));
-});
-
 it("sign out when clicking on sign in as", async () => {
   const { context, testRenderer } = createTestRenderer({
     role: GQLUSER_ROLE.COMMENTER,

src/core/client/ui/components/CallOut/CallOut.css

@@ -8,6 +8,7 @@
   box-sizing: border-box;
   border-width: 1px;
   border-style: solid;
+  word-break: break-word;
 }
 
 .colorRegular {

src/core/server/app/handlers/api/auth/local.ts

@@ -9,7 +9,7 @@ import {
 import { validate } from "talk-server/app/request/body";
 import { GQLUSER_ROLE } from "talk-server/graph/tenant/schema/__generated__/types";
 import { LocalProfile } from "talk-server/models/user";
-import { upsert } from "talk-server/services/users";
+import { insert } from "talk-server/services/users";
 import { Request } from "talk-server/types/express";
 
 export interface SignupBody {
@@ -65,7 +65,7 @@ export const signupHandler = ({
     };
 
     // Create the new user.
-    const user = await upsert(mongo, tenant, {
+    const user = await insert(mongo, tenant, {
       email,
       username,
       profiles: [profile],

src/core/server/app/handlers/api/install.ts

@@ -10,14 +10,14 @@ import { TenantInstalledAlreadyError } from "talk-server/errors";
 import { GQLUSER_ROLE } from "talk-server/graph/tenant/schema/__generated__/types";
 import { LocalProfile } from "talk-server/models/user";
 import { install, InstallTenant } from "talk-server/services/tenant";
-import { upsert, UpsertUser } from "talk-server/services/users";
+import { insert, InsertUser } from "talk-server/services/users";
 import { RequestHandler } from "talk-server/types/express";
 
 export interface TenantInstallBody {
   tenant: Omit<InstallTenant, "domain" | "locale"> & {
     locale: LanguageCode | null;
   };
-  user: Required<Pick<UpsertUser, "username" | "email"> & { password: string }>;
+  user: Required<Pick<InsertUser, "username" | "email"> & { password: string }>;
 }
 
 const TenantInstallBodySchema = Joi.object().keys({
@@ -113,7 +113,7 @@ export const installHandler = ({
     };
 
     // Create the first admin user.
-    await upsert(mongo, tenant, {
+    await insert(mongo, tenant, {
       email,
       username,
       profiles: [profile],

src/core/server/app/middleware/passport/strategies/facebook.ts

@@ -14,7 +14,7 @@ import {
   FacebookProfile,
   retrieveUserWithProfile,
 } from "talk-server/models/user";
-import { upsert } from "talk-server/services/users";
+import { insert } from "talk-server/services/users";
 
 export type FacebookStrategyOptions = OAuth2StrategyOptions;
 
@@ -71,7 +71,7 @@ export default class FacebookStrategy extends OAuth2Strategy<
         emailVerified = false;
       }
 
-      user = await upsert(this.mongo, tenant, {
+      user = await insert(this.mongo, tenant, {
         username: displayName,
         role: GQLUSER_ROLE.COMMENTER,
         email,

src/core/server/app/middleware/passport/strategies/google.ts

@@ -14,7 +14,7 @@ import {
   GoogleProfile,
   retrieveUserWithProfile,
 } from "talk-server/models/user";
-import { upsert } from "talk-server/services/users";
+import { insert } from "talk-server/services/users";
 
 export type GoogleStrategyOptions = OAuth2StrategyOptions;
 
@@ -70,7 +70,7 @@ export default class GoogleStrategy extends OAuth2Strategy<
         emailVerified = false;
       }
 
-      user = await upsert(this.mongo, tenant, {
+      user = await insert(this.mongo, tenant, {
         username: displayName,
         role: GQLUSER_ROLE.COMMENTER,
         email,

src/core/server/app/middleware/passport/strategies/jwt.ts

@@ -2,35 +2,31 @@ import jwt from "jsonwebtoken";
 import { Strategy } from "passport-strategy";
 
 import { AppOptions } from "talk-server/app";
-import {
-  JWTToken,
-  JWTVerifier,
-} from "talk-server/app/middleware/passport/strategies/verifiers/jwt";
-import {
-  SSOToken,
-  SSOVerifier,
-} from "talk-server/app/middleware/passport/strategies/verifiers/sso";
 import { TenantNotFoundError, TokenInvalidError } from "talk-server/errors";
 import { Tenant } from "talk-server/models/tenant";
 import { User } from "talk-server/models/user";
 import { extractJWTFromRequest } from "talk-server/services/jwt";
 import { Request } from "talk-server/types/express";
 
+import { JWTToken, JWTVerifier } from "./verifiers/jwt";
+import { OIDCIDToken, OIDCVerifier } from "./verifiers/oidc";
+import { SSOToken, SSOVerifier } from "./verifiers/sso";
+
 export type JWTStrategyOptions = Pick<
   AppOptions,
-  "signingConfig" | "mongo" | "redis"
+  "signingConfig" | "mongo" | "redis" | "tenantCache"
 >;
 
 /**
  * Token is the various forms of the Token that can be verified.
  */
-type Token = SSOToken | JWTToken | object | string | null;
+type Token = OIDCIDToken | SSOToken | JWTToken | object | string | null;
 
 /**
  * Verifier allows different implementations to offer ways to verify a given
  * Token.
  */
-interface Verifier<T> {
+export interface Verifier<T = Token> {
   /**
    * verify will perform the verification and return a User.
    */
@@ -50,18 +46,16 @@ interface Verifier<T> {
 export class JWTStrategy extends Strategy {
   public name = "jwt";
 
-  private verifiers: {
-    sso: Verifier<SSOToken>;
-    jwt: Verifier<JWTToken>;
-  };
+  private verifiers: Verifier[];
 
   constructor(options: JWTStrategyOptions) {
     super();
 
-    this.verifiers = {
-      sso: new SSOVerifier(options),
-      jwt: new JWTVerifier(options),
-    };
+    this.verifiers = [
+      new OIDCVerifier(options),
+      new SSOVerifier(options),
+      new JWTVerifier(options),
+    ];
   }
 
   private async verify(tokenString: string, tenant: Tenant) {
@@ -70,22 +64,11 @@ export class JWTStrategy extends Strategy {
       throw new TokenInvalidError(tokenString, "token could not be decoded");
     }
 
-    // TODO: add OIDC support.
-    // At the moment, OpenID Connect tokens are not supported here directly,
-    // instead, the default implementation redirects the user to the
-    // authorization endpoint where they login, and a redirection occurs
-    // yielding the token to us via the Authorization Code Flow. We then issue a
-    // Talk Token for that request, that the client uses after.
-
-    // Handle SSO integrations.
-    if (this.verifiers.sso.supports(token, tenant)) {
-      return this.verifiers.sso.verify(tokenString, token, tenant);
-    }
-
-    // Handle the raw JWT token.
-    if (this.verifiers.jwt.supports(token, tenant)) {
-      // Verify the token with the JWT verification strategy.
-      return this.verifiers.jwt.verify(tokenString, token, tenant);
+    // Try to verify the token.
+    for (const verifier of this.verifiers) {
+      if (verifier.supports(token, tenant)) {
+        return verifier.verify(tokenString, token, tenant);
+      }
     }
 
     // No verifier could be found.

src/core/server/app/middleware/passport/strategies/oidc/index.ts

@@ -7,15 +7,18 @@ import { Strategy } from "passport-strategy";
 
 import { validate } from "talk-server/app/request/body";
 import { reconstructURL } from "talk-server/app/url";
-import {
-  GQLOIDCAuthIntegration,
-  GQLUSER_ROLE,
-} from "talk-server/graph/tenant/schema/__generated__/types";
+import { GQLUSER_ROLE } from "talk-server/graph/tenant/schema/__generated__/types";
+import logger from "talk-server/logger";
+import { OIDCAuthIntegration } from "talk-server/models/settings";
 import { Tenant } from "talk-server/models/tenant";
-import { OIDCProfile, retrieveUserWithProfile } from "talk-server/models/user";
+import {
+  OIDCProfile,
+  retrieveUserWithProfile,
+  User,
+} from "talk-server/models/user";
 import TenantCache from "talk-server/services/tenant/cache";
 import { TenantCacheAdapter } from "talk-server/services/tenant/cache/adapter";
-import { upsert } from "talk-server/services/users";
+import { insert } from "talk-server/services/users";
 import { Request } from "talk-server/types/express";
 
 export interface Params {
@@ -85,11 +88,9 @@ const signingKeyFactory = (client: jwks.JwksClient): jwt.KeyFunction => (
   });
 };
 
-function getEnabledIntegration(
-  tenant: Tenant
-): Required<GQLOIDCAuthIntegration> {
-  // Grab the OIDC Integration.
-  const integration = tenant.auth.integrations.oidc;
+export function getEnabledIntegration(
+  integration: OIDCAuthIntegration
+): Required<OIDCAuthIntegration> {
   if (!integration.enabled) {
     // TODO: return a better error.
     throw new Error("integration not enabled");
@@ -109,7 +110,7 @@ function getEnabledIntegration(
   }
 
   // TODO: (wyattjoh) for some reason, type guards above to not allow coercion to this required type.
-  return integration as Required<GQLOIDCAuthIntegration>;
+  return integration as Required<OIDCAuthIntegration>;
 }
 
 export const OIDCIDTokenSchema = Joi.object()
@@ -135,9 +136,9 @@ export const OIDCIDTokenSchema = Joi.object()
 export async function findOrCreateOIDCUser(
   mongo: Db,
   tenant: Tenant,
-  integration: GQLOIDCAuthIntegration,
+  integration: OIDCAuthIntegration,
   token: OIDCIDToken
-) {
+): Promise<Readonly<User> | null> {
   // Unpack/validate the token content.
   const {
     sub,
@@ -160,15 +161,11 @@ export async function findOrCreateOIDCUser(
   };
 
   // Try to lookup user given their id provided in the `sub` claim.
-  let user = await retrieveUserWithProfile(mongo, tenant.id, {
-    // NOTE: (wyattjoh) as the current requirements do not allow multiple OIDC integrations, we are only getting the profile based on the OIDC provider.
-    type: "oidc",
-    id: sub,
-  });
+  let user = await retrieveUserWithProfile(mongo, tenant.id, profile);
   if (!user) {
     if (!integration.allowRegistration) {
       // Registration is disabled, so we can't create the user user here.
-      return;
+      return null;
     }
 
     // FIXME: implement rules.
@@ -177,7 +174,7 @@ export async function findOrCreateOIDCUser(
     const username = preferred_username || nickname || name;
 
     // Create the new user, as one didn't exist before!
-    user = await upsert(mongo, tenant, {
+    user = await insert(mongo, tenant, {
       username,
       role: GQLUSER_ROLE.COMMENTER,
       email,
@@ -192,6 +189,47 @@ export async function findOrCreateOIDCUser(
   return user;
 }
 
+export function findOrCreateOIDCUserWithToken(
+  mongo: Db,
+  tenant: Tenant,
+  client: JwksClient,
+  integration: OIDCAuthIntegration,
+  token: string
+) {
+  return new Promise<Readonly<User> | null>((resolve, reject) => {
+    logger.trace({ tenantID: tenant.id }, "verifying oidc id_token");
+    jwt.verify(
+      token,
+      signingKeyFactory(client),
+      {
+        issuer: integration.issuer,
+      },
+      async (err, decoded) => {
+        logger.trace(
+          { tenantID: tenant.id },
+          "finished verifying oidc id_token"
+        );
+        if (err) {
+          // TODO: wrap error?
+          return reject(err);
+        }
+
+        try {
+          const user = await findOrCreateOIDCUser(
+            mongo,
+            tenant,
+            integration,
+            decoded as OIDCIDToken
+          );
+          return resolve(user);
+        } catch (err) {
+          return reject(err);
+        }
+      }
+    );
+  });
+}
+
 /**
  * OIDC_SCOPE is the set of scopes requested for users signing up via OIDC.
  */
@@ -218,7 +256,7 @@ export default class OIDCStrategy extends Strategy {
   private lookupJWKSClient(
     req: Request,
     tenantID: string,
-    oidc: Required<GQLOIDCAuthIntegration>
+    oidc: Required<OIDCAuthIntegration>
   ): jwks.JwksClient {
     let tenantIntegration = this.cache.get(tenantID);
     if (!tenantIntegration) {
@@ -249,7 +287,7 @@ export default class OIDCStrategy extends Strategy {
     return tenantIntegration.jwksClient;
   }
 
-  private userAuthenticatedCallback = (
+  private userAuthenticatedCallback = async (
     req: Request,
     accessToken: string, // ignore the access token, we don't use it.
     refreshToken: string, // ignore the refresh token, we don't use it.
@@ -274,9 +312,9 @@ export default class OIDCStrategy extends Strategy {
 
     // Get the integration from the tenant. If needed, it will be used to create
     // a new strategy.
-    let integration: Required<GQLOIDCAuthIntegration>;
+    let integration: Required<OIDCAuthIntegration>;
     try {
-      integration = getEnabledIntegration(tenant);
+      integration = getEnabledIntegration(tenant.auth.integrations.oidc);
     } catch (err) {
       // TODO: wrap error?
       return done(err);
@@ -286,36 +324,23 @@ export default class OIDCStrategy extends Strategy {
     const client = this.lookupJWKSClient(req, tenant.id, integration);
 
     // Verify that the id_token is valid or not.
-    jwt.verify(
-      id_token,
-      signingKeyFactory(client),
-      {
-        issuer: integration.issuer,
-      },
-      async (err, decoded) => {
-        if (err) {
-          // TODO: wrap error?
-          return done(err);
-        }
-
-        try {
-          const user = await findOrCreateOIDCUser(
-            this.mongo,
-            tenant,
-            integration,
-            decoded as OIDCIDToken
-          );
-          return done(null, user);
-        } catch (err) {
-          return done(err);
-        }
-      }
-    );
+    try {
+      const user = await findOrCreateOIDCUserWithToken(
+        this.mongo,
+        tenant,
+        client,
+        integration,
+        id_token
+      );
+      return done(null, user || undefined);
+    } catch (err) {
+      return done(err);
+    }
   };
 
   private createStrategy(
     req: Request,
-    integration: Required<GQLOIDCAuthIntegration>
+    integration: Required<OIDCAuthIntegration>
   ): OAuth2Strategy {
     const { clientID, clientSecret, authorizationURL, tokenURL } = integration;
 
@@ -346,7 +371,7 @@ export default class OIDCStrategy extends Strategy {
 
     // Get the integration from the tenant. If needed, it will be used to create
     // a new strategy.
-    const integration = getEnabledIntegration(tenant);
+    const integration = getEnabledIntegration(tenant.auth.integrations.oidc);
 
     // Try to get the Tenant's cached integrations.
     let tenantIntegration = this.cache.get(tenant.id);

src/core/server/app/middleware/passport/strategies/verifiers/jwt.ts

@@ -8,6 +8,8 @@ import { Tenant } from "talk-server/models/tenant";
 import { retrieveUser } from "talk-server/models/user";
 import { checkJWTRevoked, JWTSigningConfig } from "talk-server/services/jwt";
 
+import { Verifier } from "../jwt";
+
 export interface JWTToken {
   /**
    * jti is the Token identifier. With normal login tokens, this is a randomly
@@ -72,7 +74,7 @@ export interface JWTVerifierOptions {
   redis: Redis;
 }
 
-export class JWTVerifier {
+export class JWTVerifier implements Verifier<JWTToken> {
   private signingConfig: JWTSigningConfig;
   private mongo: Db;
   private redis: Redis;

src/core/server/app/middleware/passport/strategies/verifiers/oidc.ts

@@ -0,0 +1,69 @@
+import jwks, { JwksClient } from "jwks-rsa";
+import { Db } from "mongodb";
+
+import { AppOptions } from "talk-server/app";
+import { Tenant } from "talk-server/models/tenant";
+import { TenantCacheAdapter } from "talk-server/services/tenant/cache/adapter";
+
+import logger from "talk-server/logger";
+import { Verifier } from "../jwt";
+import {
+  findOrCreateOIDCUserWithToken,
+  getEnabledIntegration,
+  isOIDCToken,
+  OIDCIDToken,
+} from "../oidc";
+
+export type OIDCIDToken = OIDCIDToken;
+
+export type OIDCVerifierOptions = Pick<
+  AppOptions,
+  "mongo" | "redis" | "tenantCache"
+>;
+
+export class OIDCVerifier implements Verifier<OIDCIDToken> {
+  private mongo: Db;
+  private cache: TenantCacheAdapter<JwksClient>;
+
+  constructor({ mongo, tenantCache }: OIDCVerifierOptions) {
+    this.mongo = mongo;
+    this.cache = new TenantCacheAdapter(tenantCache);
+  }
+
+  public async verify(tokenString: string, token: OIDCIDToken, tenant: Tenant) {
+    // Ensure that the integration is enabled.
+    const integration = getEnabledIntegration(tenant.auth.integrations.oidc);
+
+    // Grab the JWKS client to verify the SSO ID token.
+    let client = this.cache.get(tenant.id);
+    if (!client) {
+      logger.trace({ tenantID: tenant.id }, "jwks client not cached");
+      client = jwks({
+        jwksUri: integration.jwksURI,
+      });
+
+      this.cache.set(tenant.id, client);
+    } else {
+      logger.trace({ tenantID: tenant.id }, "jwks client cached");
+    }
+
+    return findOrCreateOIDCUserWithToken(
+      this.mongo,
+      tenant,
+      client,
+      integration,
+      tokenString
+    );
+  }
+
+  public supports(
+    token: OIDCIDToken | object,
+    tenant: Tenant
+  ): token is OIDCIDToken {
+    return (
+      tenant.auth.integrations.oidc.enabled &&
+      Boolean(tenant.auth.integrations.oidc.jwksURI) &&
+      isOIDCToken(token)
+    );
+  }
+}

src/core/server/app/middleware/passport/strategies/verifiers/sso.ts

@@ -9,7 +9,9 @@ import {
 } from "talk-server/graph/tenant/schema/__generated__/types";
 import { Tenant } from "talk-server/models/tenant";
 import { retrieveUserWithProfile, SSOProfile } from "talk-server/models/user";
-import { upsert } from "talk-server/services/users";
+import { insert } from "talk-server/services/users";
+
+import { Verifier } from "../jwt";
 
 export interface SSOStrategyOptions {
   mongo: Db;
@@ -69,7 +71,7 @@ export async function findOrCreateSSOUser(
     // FIXME: (wyattjoh) implement rules! Not all users should be able to create an account via this method.
 
     // Create the new user, as one didn't exist before!
-    user = await upsert(mongo, tenant, {
+    user = await insert(mongo, tenant, {
       username,
       role: GQLUSER_ROLE.COMMENTER,
       email,
@@ -109,7 +111,7 @@ export interface SSOVerifierOptions {
   mongo: Db;
 }
 
-export class SSOVerifier {
+export class SSOVerifier implements Verifier<SSOToken> {
   private mongo: Db;
 
   constructor({ mongo }: SSOVerifierOptions) {

src/core/server/index.ts

@@ -123,7 +123,10 @@ class Server {
     // Create the database indexes if it isn't disabled.
     if (!this.config.get("disable_mongodb_autoindexing")) {
       // Setup the database indexes.
+      logger.info("mongodb autoindexing is enabled, starting indexing");
       await ensureIndexes(this.mongo);
+    } else {
+      logger.info("mongodb autoindexing is disabled, skipping indexing");
     }
 
     // Launch all of the job processors.

src/core/server/models/user.ts

@@ -1,5 +1,5 @@
 import bcrypt from "bcryptjs";
-import { Db } from "mongodb";
+import { Db, MongoError } from "mongodb";
 import uuid from "uuid";
 
 import { Omit, Sub } from "talk-common/types";
@@ -17,7 +17,7 @@ import {
   createConnectionOrderVariants,
   createIndexFactory,
 } from "talk-server/models/helpers/indexing";
-import Query, { FilterQuery } from "talk-server/models/helpers/query";
+import Query from "talk-server/models/helpers/query";
 import { TenantResource } from "talk-server/models/tenant";
 import {
   Connection,
@@ -97,12 +97,15 @@ export async function createUserIndexes(mongo: Db) {
   // UNIQUE { profiles.type, profiles.id }
   await createIndex(
     { tenantID: 1, "profiles.type": 1, "profiles.id": 1 },
+    { unique: true, partialFilterExpression: { profiles: { $exists: true } } }
+  );
+
+  // { profiles }
+  await createIndex(
+    { tenantID: 1, profiles: 1, email: 1 },
     {
-      unique: true,
-      // We're filtering by the first entry in the profiles array to ensure we
-      // only enforce uniqueness when the profiles array has at least a single
-      // profile.
-      partialFilterExpression: { "profiles.0": { $exists: true } },
+      partialFilterExpression: { profiles: { $exists: true } },
+      background: true,
     }
   );
 
@@ -141,15 +144,15 @@ function hashPassword(password: string): Promise<string> {
   return bcrypt.hash(password, 10);
 }
 
-export type UpsertUserInput = Omit<
+export type InsertUserInput = Omit<
   User,
   "id" | "tenantID" | "tokens" | "createdAt"
 >;
 
-export async function upsertUser(
+export async function insertUser(
   mongo: Db,
   tenantID: string,
-  input: UpsertUserInput
+  input: InsertUserInput
 ) {
   const now = new Date();
 
@@ -158,13 +161,18 @@ export async function upsertUser(
 
   // default are the properties set by the application when a new user is
   // created.
-  const defaults: Sub<User, UpsertUserInput> = {
+  const defaults: Sub<User, InsertUserInput> = {
     id,
     tenantID,
     tokens: [],
     createdAt: now,
   };
 
+  // Guard against empty login profiles (they need some way to login).
+  if (input.profiles.length === 0) {
+    throw new Error("users require at least one profile");
+  }
+
   // Mutate the profiles to ensure we mask handle any secrets.
   const profiles: Profile[] = [];
   for (let profile of input.profiles) {
@@ -189,52 +197,22 @@ export async function upsertUser(
     profiles,
   };
 
-  // Create a query that will utilize a findOneAndUpdate to facilitate an upsert
-  // operation to ensure no user has the same profile and/or email address. If
-  // any user is found to have the same profile as any of the profiles specified
-  // in the new user object, then we should error here.
-  const filter = createUpsertUserFilter(user);
+  try {
+    // Insert it into the database. This may throw an error.
+    await collection(mongo).insert(user);
+  } catch (err) {
+    // Evaluate the error, if it is in regards to violating the unique index,
+    // then return a duplicate Story error.
+    if (err instanceof MongoError && err.code === 11000) {
+      throw new DuplicateUserError();
+    }
 
-  // Create the upsert/update operation.
-  const update: { $setOnInsert: Readonly<User> } = {
-    $setOnInsert: user,
-  };
-
-  // Insert it into the database. This may throw an error.
-  const result = await collection(mongo).findOneAndUpdate(filter, update, {
-    // We are using this to create a user, so we need to upsert it.
-    upsert: true,
-
-    // False to return the updated document instead of the original document.
-    // This lets us detect if the document was updated or not.
-    returnOriginal: false,
-  });
-
-  // Check to see if this was a new user that was upserted, or one was found
-  // that matched existing records. We are sure here that the record exists
-  // because we're returning the updated document and performing an upsert
-  // operation.
-  if (result.value!.id !== id) {
-    throw new DuplicateUserError();
+    throw err;
   }
 
-  return result.value!;
+  return user;
 }
 
-const createUpsertUserFilter = (user: Readonly<User>) => {
-  const query: FilterQuery<User> = {
-    // Query by the profiles if the user is being created with one.
-    $or: user.profiles.map(profile => ({ profiles: { $elemMatch: profile } })),
-  };
-
-  if (user.email) {
-    // Query by the email address if the user is being created with one.
-    query.$or.push({ email: user.email });
-  }
-
-  return query;
-};
-
 export async function retrieveUser(mongo: Db, tenantID: string, id: string) {
   return collection(mongo).findOne({ tenantID, id });
 }

src/core/server/services/users/index.ts

@@ -26,6 +26,8 @@ import { Tenant } from "talk-server/models/tenant";
 import {
   createUserToken,
   deactivateUserToken,
+  insertUser,
+  InsertUserInput,
   LocalProfile,
   setUserEmail,
   setUserLocalProfile,
@@ -35,8 +37,6 @@ import {
   updateUserPassword,
   updateUserRole,
   updateUserUsername,
-  upsertUser,
-  UpsertUserInput,
   User,
 } from "talk-server/models/user";
 
@@ -51,7 +51,7 @@ import { JWTSigningConfig, signPATString } from "../jwt";
  * @param username the username to be tested
  */
 function validateUsername(tenant: Tenant, username: string) {
-  // TODO: replace these static regex/length with database options in the Tenant eventually
+  // FIXME: replace these static regex/length with database options in the Tenant eventually
 
   if (!USERNAME_REGEX.test(username)) {
     throw new UsernameContainsInvalidCharactersError();
@@ -104,16 +104,16 @@ function validateEmail(tenant: Tenant, email: string) {
   }
 }
 
-export type UpsertUser = UpsertUserInput;
+export type InsertUser = InsertUserInput;
 
 /**
- * upsert will upsert the User into the database for the Tenant.
+ * insert will upsert the User into the database for the Tenant.
  *
  * @param mongo mongo database to interact with
  * @param tenant Tenant where the User will be added to
  * @param input the input for creating the User
  */
-export async function upsert(mongo: Db, tenant: Tenant, input: UpsertUser) {
+export async function insert(mongo: Db, tenant: Tenant, input: InsertUser) {
   if (input.username) {
     validateUsername(tenant, input.username);
   }
@@ -134,7 +134,7 @@ export async function upsert(mongo: Db, tenant: Tenant, input: UpsertUser) {
     }
   }
 
-  const user = await upsertUser(mongo, tenant.id, input);
+  const user = await insertUser(mongo, tenant.id, input);
 
   return user;
 }