Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CORL-1238] Duplicate User Race #3068

Merged
merged 3 commits into from Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/core/server/errors/index.ts
Expand Up @@ -252,8 +252,8 @@ export class StoryURLInvalidError extends CoralError {
}

export class DuplicateUserError extends CoralError {
constructor() {
super({ code: ERROR_CODES.DUPLICATE_USER });
constructor(cause?: Error) {
super({ cause, code: ERROR_CODES.DUPLICATE_USER });
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/core/server/models/user/user.ts
Expand Up @@ -637,7 +637,9 @@ export async function findOrCreateUser(
if (err.errmsg && err.errmsg.includes("tenantID_1_email_1")) {
throw new DuplicateEmailError(input.email!);
}
throw new DuplicateUserError();

// Some other error occured.
throw new DuplicateUserError(err);
}

throw err;
Expand Down Expand Up @@ -666,7 +668,8 @@ export async function createUser(
throw new DuplicateEmailError(input.email!);
}

throw new DuplicateUserError();
// Some other error occured.
throw new DuplicateUserError(err);
}

throw err;
Expand Down
40 changes: 22 additions & 18 deletions src/core/server/services/users/users.ts
Expand Up @@ -138,44 +138,48 @@ export async function findOrCreate(
// Validate the input.
validateFindOrCreateUserInput(input, options);

let user: Readonly<User>;
let wasUpserted: boolean;

try {
const result = await findOrCreateUser(mongo, tenant.id, input, now);
user = result.user;
wasUpserted = result.wasUpserted;
// Try to find or create the user.
const { user } = await findOrCreateUser(mongo, tenant.id, input, now);

return user;
} catch (err) {
// If this error is related to a duplicate user error, we might have
// encountered a race related to the unique index. We should try once more
// to perform the operation.
if (err instanceof DuplicateUserError) {
// Retry the operation once more, if this operation fails, the error will
// exit this function.
const { user } = await findOrCreateUser(mongo, tenant.id, input, now);

return user;
}

// If this is an error related to a duplicate email, we might be in a
// position where the user can link their accounts. This can only occur if
// the tenant has both local and another social profile enabled.
if (err instanceof DuplicateEmailError && linkUsersAvailable(tenant)) {
// Pull the email address out of the input, and re-try creating the user
// given that.
// given that. We need to pull the verified property out because we don't
// want to have that embedded in the `...rest` object.
const { email, emailVerified, ...rest } = input;

// Create the user again this time, but associate the duplicate email to
// the user account.
const result = await findOrCreateUser(
const { user } = await findOrCreateUser(
mongo,
tenant.id,
{ ...rest, duplicateEmail: email },
now
);

user = result.user;
wasUpserted = result.wasUpserted;
} else {
throw err;
return user;
}
}

if (wasUpserted) {
// TODO: (wyattjoh) emit that a user was created
// The error wasn't related to a duplicate user or duplicate email error,
// so just re-throw that error again.
throw err;
}

// TODO: (wyattjoh) evaluate the tenant to determine if we should send the verification email.
return user;
}

export type CreateUser = FindOrCreateUserInput;
Expand Down