diff --git a/src/core/server/app/handlers/api/auth/local/signup.ts b/src/core/server/app/handlers/api/auth/local/signup.ts index a09ac635d..82877906b 100644 --- a/src/core/server/app/handlers/api/auth/local/signup.ts +++ b/src/core/server/app/handlers/api/auth/local/signup.ts @@ -8,7 +8,7 @@ import { RequestLimiter } from "coral-server/app/request/limiter"; import { IntegrationDisabled } from "coral-server/errors"; import { GQLUSER_ROLE } from "coral-server/graph/tenant/schema/__generated__/types"; import { LocalProfile, User } from "coral-server/models/user"; -import { insert } from "coral-server/services/users"; +import { create } from "coral-server/services/users"; import { sendConfirmationEmail } from "coral-server/services/users/auth"; import { RequestHandler } from "coral-server/types/express"; @@ -82,13 +82,13 @@ export const signupHandler = ({ }; // Create the new user. - const user = await insert( + const user = await create( mongo, tenant, { email, username, - profiles: [profile], + profile, // New users signing up via local auth will have the commenter role to // start with. role: GQLUSER_ROLE.COMMENTER, diff --git a/src/core/server/app/handlers/api/install.ts b/src/core/server/app/handlers/api/install.ts index ef3751808..d4f9b1a35 100644 --- a/src/core/server/app/handlers/api/install.ts +++ b/src/core/server/app/handlers/api/install.ts @@ -9,14 +9,14 @@ import { TenantInstalledAlreadyError } from "coral-server/errors"; import { GQLUSER_ROLE } from "coral-server/graph/tenant/schema/__generated__/types"; import { LocalProfile } from "coral-server/models/user"; import { install, InstallTenant } from "coral-server/services/tenant"; -import { insert, InsertUser } from "coral-server/services/users"; +import { create, CreateUser } from "coral-server/services/users"; import { RequestHandler } from "coral-server/types/express"; export interface TenantInstallBody { tenant: Omit & { locale: LanguageCode | null; }; - user: Required & { password: string }>; + user: Required & { password: string }>; } const TenantInstallBodySchema = Joi.object().keys({ @@ -120,13 +120,13 @@ export const installHandler = ({ }; // Create the first admin user. - await insert( + await create( mongo, tenant, { email, username, - profiles: [profile], + profile, role: GQLUSER_ROLE.ADMIN, }, req.coral.now diff --git a/src/core/server/app/middleware/passport/strategies/facebook.ts b/src/core/server/app/middleware/passport/strategies/facebook.ts index 76933c7fc..60212e098 100644 --- a/src/core/server/app/middleware/passport/strategies/facebook.ts +++ b/src/core/server/app/middleware/passport/strategies/facebook.ts @@ -14,7 +14,7 @@ import { FacebookProfile, retrieveUserWithProfile, } from "coral-server/models/user"; -import { insert } from "coral-server/services/users"; +import { findOrCreate } from "coral-server/services/users"; export type FacebookStrategyOptions = OAuth2StrategyOptions; @@ -72,7 +72,7 @@ export default class FacebookStrategy extends OAuth2Strategy< emailVerified = false; } - user = await insert( + user = await findOrCreate( this.mongo, tenant, { @@ -80,7 +80,7 @@ export default class FacebookStrategy extends OAuth2Strategy< email, emailVerified, avatar, - profiles: [profile], + profile, }, now ); diff --git a/src/core/server/app/middleware/passport/strategies/google.ts b/src/core/server/app/middleware/passport/strategies/google.ts index 4111502d3..7bd0a829e 100644 --- a/src/core/server/app/middleware/passport/strategies/google.ts +++ b/src/core/server/app/middleware/passport/strategies/google.ts @@ -14,7 +14,7 @@ import { GoogleProfile, retrieveUserWithProfile, } from "coral-server/models/user"; -import { insert } from "coral-server/services/users"; +import { findOrCreate } from "coral-server/services/users"; export type GoogleStrategyOptions = OAuth2StrategyOptions; @@ -71,7 +71,7 @@ export default class GoogleStrategy extends OAuth2Strategy< emailVerified = false; } - user = await insert( + user = await findOrCreate( this.mongo, tenant, { @@ -79,7 +79,7 @@ export default class GoogleStrategy extends OAuth2Strategy< email, emailVerified, avatar, - profiles: [profile], + profile, }, now ); diff --git a/src/core/server/app/middleware/passport/strategies/oidc/index.ts b/src/core/server/app/middleware/passport/strategies/oidc/index.ts index b77c82770..6d52e0316 100644 --- a/src/core/server/app/middleware/passport/strategies/oidc/index.ts +++ b/src/core/server/app/middleware/passport/strategies/oidc/index.ts @@ -21,7 +21,7 @@ import { import { AsymmetricSigningAlgorithm } from "coral-server/services/jwt"; import TenantCache from "coral-server/services/tenant/cache"; import { TenantCacheAdapter } from "coral-server/services/tenant/cache/adapter"; -import { insert } from "coral-server/services/users"; +import { findOrCreate } from "coral-server/services/users"; import { validateUsername } from "coral-server/services/users/helpers"; import { Request } from "coral-server/types/express"; @@ -181,7 +181,7 @@ export async function findOrCreateOIDCUser( } // Create the new user, as one didn't exist before! - user = await insert( + user = await findOrCreate( mongo, tenant, { @@ -190,7 +190,7 @@ export async function findOrCreateOIDCUser( email, emailVerified: email_verified, avatar: picture, - profiles: [profile], + profile, }, now ); diff --git a/src/core/server/app/middleware/passport/strategies/verifiers/sso.ts b/src/core/server/app/middleware/passport/strategies/verifiers/sso.ts index d77d6a4df..a06dc94b0 100644 --- a/src/core/server/app/middleware/passport/strategies/verifiers/sso.ts +++ b/src/core/server/app/middleware/passport/strategies/verifiers/sso.ts @@ -14,7 +14,7 @@ import { SSOProfile, updateUserFromSSO, } from "coral-server/models/user"; -import { insert } from "coral-server/services/users"; +import { findOrCreate } from "coral-server/services/users"; import { getSSOProfile, @@ -127,7 +127,7 @@ export async function findOrCreateSSOUser( }; // Create the new user, as one didn't exist before! - user = await insert( + user = await findOrCreate( mongo, tenant, { @@ -136,7 +136,8 @@ export async function findOrCreateSSOUser( role: role || GQLUSER_ROLE.COMMENTER, badges, email, - profiles: [profile], + emailVerified: true, + profile, }, now ); diff --git a/src/core/server/models/user/user.ts b/src/core/server/models/user/user.ts index 024dcea75..89c1599a6 100644 --- a/src/core/server/models/user/user.ts +++ b/src/core/server/models/user/user.ts @@ -468,29 +468,32 @@ function hashPassword(password: string): Promise { return bcrypt.hash(password, 10); } -export type InsertUserInput = Omit< - User, - | "id" - | "tenantID" - | "tokens" - | "status" - | "ignoredUsers" - | "notifications" - | "digests" - | "emailVerificationID" - | "createdAt" -> & - Partial>; +export interface FindOrCreateUserInput { + id?: string; + username?: string; + avatar?: string; + email?: string; + badges?: string[]; + emailVerified?: boolean; + role: GQLUSER_ROLE; + profile: Profile; +} -export async function insertUser( - mongo: Db, +/** + * findOrCreateUserInput converts the FindOrCreateUserInput input into aUse. + * + * @param tenantID ID of the Tenant to create the user for + * @param input the input for creating a User + * @param now the current date + */ +async function findOrCreateUserInput( tenantID: string, - { id = uuid.v4(), ...input }: InsertUserInput, - now = new Date() -) { + { id = uuid.v4(), profile, ...input }: FindOrCreateUserInput, + now: Date +): Promise> { // default are the properties set by the application when a new user is // created. - const defaults: Sub = { + const defaults: Sub = { tenantID, tokens: [], ignoredUsers: [], @@ -508,11 +511,13 @@ export async function insertUser( onStaffReplies: false, digestFrequency: GQLDIGEST_FREQUENCY.NONE, }, + profiles: [], digests: [], createdAt: now, }; if (input.username) { + // Add the username history to the user. defaults.status.username.history.push({ id: uuid.v4(), username: input.username, @@ -521,39 +526,54 @@ export async function insertUser( }); } - // 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) { - switch (profile.type) { - case "local": - // Hash the user's password with bcrypt. - const password = await hashPassword(profile.password); - profile = { - ...profile, - password, - }; - break; - } - // Save a copy. - profiles.push(profile); + switch (profile.type) { + case "local": + // Hash the user's password with bcrypt. + const password = await hashPassword(profile.password); + defaults.profiles.push({ ...profile, password }); + break; + default: + // Push the profile onto the User. + defaults.profiles.push(profile); + break; } // Merge the defaults and the input together. - const user: Readonly = { + return { ...defaults, ...input, id, - profiles, }; +} + +export async function findOrCreateUser( + mongo: Db, + tenantID: string, + input: FindOrCreateUserInput, + now: Date +) { + const user = await findOrCreateUserInput(tenantID, input, now); try { - // Insert it into the database. This may throw an error. - await collection(mongo).insert(user); + await collection(mongo).findOneAndUpdate( + { + tenantID, + profiles: { + $elemMatch: { + id: input.profile.id, + type: input.profile.type, + }, + }, + }, + { $setOnInsert: user }, + { + // False to return the updated document instead of the original + // document. + returnOriginal: false, + upsert: true, + } + ); } catch (err) { // Evaluate the error, if it is in regards to violating the unique index, // then return a duplicate User error. @@ -571,6 +591,37 @@ export async function insertUser( return user; } +export type CreateUserInput = FindOrCreateUserInput; + +export async function createUser( + mongo: Db, + tenantID: string, + input: CreateUserInput, + now: Date +) { + const user = await findOrCreateUserInput(tenantID, input, now); + + try { + // Insert it into the database. This may throw an error. + await collection(mongo).insertOne(user); + } catch (err) { + // Evaluate the error, if it is in regards to violating the unique index, + // then return a duplicate User error. + if (err instanceof MongoError && err.code === 11000) { + // Check if duplicate index was about the email. + if (err.errmsg && err.errmsg.includes("tenantID_1_email_1")) { + throw new DuplicateEmailError(input.email!); + } + + throw new DuplicateUserError(); + } + + throw err; + } + + return user; +} + export async function retrieveUser(mongo: Db, tenantID: string, id: string) { return collection(mongo).findOne({ tenantID, id }); } @@ -581,15 +632,15 @@ export async function retrieveManyUsers( ids: string[] ) { const cursor = await collection(mongo).find({ + tenantID, id: { $in: ids, }, - tenantID, }); const users = await cursor.toArray(); - return ids.map(id => users.find(comment => comment.id === id) || null); + return ids.map(id => users.find(user => user.id === id) || null); } export async function retrieveUserWithProfile( diff --git a/src/core/server/queue/Task.ts b/src/core/server/queue/Task.ts index 3eeb02ea7..93f46d1f7 100644 --- a/src/core/server/queue/Task.ts +++ b/src/core/server/queue/Task.ts @@ -70,7 +70,10 @@ export default class Task { */ public process() { this.queue.process(async (job: Job) => { - const log = this.log.child({ jobID: job.id }, true); + const log = this.log.child( + { jobID: job.id, attemptsMade: job.attemptsMade }, + true + ); log.trace("processing job from queue"); diff --git a/src/core/server/services/users/auth/invite.ts b/src/core/server/services/users/auth/invite.ts index 4b40dae69..eb7424794 100644 --- a/src/core/server/services/users/auth/invite.ts +++ b/src/core/server/services/users/auth/invite.ts @@ -14,7 +14,7 @@ import { } from "coral-server/models/invite"; import { Tenant } from "coral-server/models/tenant"; import { - insertUser, + createUser, LocalProfile, retrieveUserWithEmail, User, @@ -310,14 +310,14 @@ export async function redeem( }; // Create the new user based on the invite. - const user = await insertUser( + const user = await createUser( mongo, tenant.id, { username, email, emailVerified: true, // Verified because the invite link was clicked. - profiles: [profile], + profile, role, }, now diff --git a/src/core/server/services/users/index.ts b/src/core/server/services/users/index.ts index cf96ee969..7b08fff13 100644 --- a/src/core/server/services/users/index.ts +++ b/src/core/server/services/users/index.ts @@ -34,11 +34,12 @@ import { clearDeletionDate, consolidateUserBanStatus, consolidateUserSuspensionStatus, + createUser, createUserToken, deactivateUserToken, + findOrCreateUser, + FindOrCreateUserInput, ignoreUser, - insertUser, - InsertUserInput, NotificationSettingsInput, removeActiveUserSuspensions, removeUserBan, @@ -76,28 +77,65 @@ import { } from "./download/token"; import { validateEmail, validatePassword, validateUsername } from "./helpers"; -export type InsertUser = InsertUserInput; - -/** - * 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 insert( - mongo: Db, - tenant: Tenant, - input: InsertUser, - now = new Date() -) { +function validateFindOrCreateUserInput(input: FindOrCreateUser) { if (input.username) { validateUsername(input.username); } if (input.email) { validateEmail(input.email); + } + const localProfile = getLocalProfile({ profiles: [input.profile] }); + if (localProfile) { + validateEmail(localProfile.id); + validatePassword(localProfile.password); + + if (input.email !== localProfile.id) { + throw new Error("email addresses don't match profile"); + } + } +} + +export type FindOrCreateUser = FindOrCreateUserInput; + +export async function findOrCreate( + mongo: Db, + tenant: Tenant, + input: FindOrCreateUser, + now: Date +) { + // Validate the input. + validateFindOrCreateUserInput(input); + + const user = await findOrCreateUser(mongo, tenant.id, input, now); + + // TODO: (wyattjoh) evaluate the tenant to determine if we should send the verification email. + + return user; +} + +export type CreateUser = FindOrCreateUserInput; + +export async function create( + mongo: Db, + tenant: Tenant, + input: CreateUser, + now: Date +) { + // Validate the input. + validateFindOrCreateUserInput(input); + + if (input.id) { + // Try to check to see if there is a user with the same ID before we try to + // create the user again. + const alreadyFoundUser = await retrieveUser(mongo, tenant.id, input.id); + if (alreadyFoundUser) { + throw new DuplicateUserError(); + } + } + + if (input.email) { // Try to lookup the user to see if this user already has an account if they // do, we can short circuit the database index hit. const alreadyFoundUser = await retrieveUserWithEmail( @@ -110,37 +148,9 @@ export async function insert( } } - if (input.id) { - // Try to check to see if there is a user with the same ID before we try to - // create the user again. - const alreadyFoundUser = await retrieveUser(mongo, tenant.id, input.id); - if (alreadyFoundUser) { - throw new DuplicateUserError(); - } - } + const user = await createUser(mongo, tenant.id, input, now); - const localProfile = getLocalProfile(input); - if (localProfile) { - validateEmail(localProfile.id); - validatePassword(localProfile.password); - - if (input.email !== localProfile.id) { - throw new Error("email addresses don't match profile"); - } - } - - const user = await insertUser(mongo, tenant.id, input, now); - - // // TODO: (wyattjoh) evaluate the tenant to determine if we should send the verification email. - // if (localProfile && user.email) { - // if (mailer) { - // // // Send the email confirmation email. - // // await sendConfirmationEmail(mongo, mailer, tenant, user, user.email); - // } else { - // // FIXME: (wyattjoh) extract the local profile based inserts into another function. - // throw new Error("local profile was provided, but the mailer was not"); - // } - // } + // TODO: (wyattjoh) evaluate the tenant to determine if we should send the verification email. return user; }