diff --git a/src/core/client/auth/test/__snapshots__/signUp.spec.tsx.snap b/src/core/client/auth/test/__snapshots__/signUp.spec.tsx.snap index 4897b8d54..002015cd5 100644 --- a/src/core/client/auth/test/__snapshots__/signUp.spec.tsx.snap +++ b/src/core/client/auth/test/__snapshots__/signUp.spec.tsx.snap @@ -1201,7 +1201,7 @@ exports[`checks for too long username 1`] = ` warning - Usernames cannot be longer than ⁨20⁩ characters. + Usernames cannot be longer than ⁨30⁩ characters. diff --git a/src/core/client/framework/lib/validation.tsx b/src/core/client/framework/lib/validation.tsx index 3021b14e8..efbe738c3 100644 --- a/src/core/client/framework/lib/validation.tsx +++ b/src/core/client/framework/lib/validation.tsx @@ -1,4 +1,14 @@ import { ReactNode } from "react"; + +import { + EMAIL_REGEX, + PASSWORD_MIN_LENGTH, + URL_REGEX, + USERNAME_MAX_LENGTH, + USERNAME_MIN_LENGTH, + USERNAME_REGEX, +} from "talk-common/helpers/validate"; + import { EMAILS_DO_NOT_MATCH, INVALID_CHARACTERS, @@ -46,7 +56,7 @@ export const required = createValidator(v => !!v, VALIDATION_REQUIRED()); * validateEmail is a Validator that checks that the value is an email. */ export const validateEmail = createValidator( - v => /^.+@.+\..+$/.test(v), + v => EMAIL_REGEX.test(v), INVALID_EMAIL() ); @@ -54,7 +64,7 @@ export const validateEmail = createValidator( * validateUsernameCharacters is a Validator that checks that the username only contains valid characters. */ export const validateUsernameCharacters = createValidator( - v => /^[a-zA-Z0-9_.]+$/.test(v), + v => USERNAME_REGEX.test(v), INVALID_CHARACTERS() ); @@ -62,10 +72,7 @@ export const validateUsernameCharacters = createValidator( * validateURL is a Validator that checks that the URL only contains valid characters. */ export const validateURL = createValidator( - v => - /^(http:\/\/www\.|https:\/\/www\.|http:\/\/|https:\/\/)[a-zA-Z0-9]+([\-\.]{1}[a-zA-Z0-9]+)*\.[a-zA-Z]{2,5}(:[0-9]{1,5})?(\/.*)?$/.test( - v - ), + v => URL_REGEX.test(v), INVALID_URL() ); @@ -73,16 +80,16 @@ export const validateURL = createValidator( * validateUsernameMinLength is a Validator that checks that the username has a min length of characters */ export const validateUsernameMinLength = createValidator( - v => v.length >= 3, - USERNAME_TOO_SHORT(3) + v => v.length >= USERNAME_MIN_LENGTH, + USERNAME_TOO_SHORT(USERNAME_MIN_LENGTH) ); /** * validateUsernameMaxLength is a Validator that checks that the username has a max length of characters */ export const validateUsernameMaxLength = createValidator( - v => v.length <= 20, - USERNAME_TOO_LONG(20) + v => v.length <= USERNAME_MAX_LENGTH, + USERNAME_TOO_LONG(USERNAME_MAX_LENGTH) ); /** @@ -98,8 +105,8 @@ export const validateUsername = composeValidators( * validateUsername is a Validator that checks that the value is a valid username. */ export const validatePassword = createValidator( - v => v.length >= 8, - PASSWORD_TOO_SHORT(8) + v => v.length >= PASSWORD_MIN_LENGTH, + PASSWORD_TOO_SHORT(PASSWORD_MIN_LENGTH) ); /** diff --git a/src/core/common/helpers/validate.ts b/src/core/common/helpers/validate.ts new file mode 100644 index 000000000..cde19e11c --- /dev/null +++ b/src/core/common/helpers/validate.ts @@ -0,0 +1,17 @@ +export const USERNAME_REGEX = new RegExp(/^[a-zA-Z0-9_.]+$/); +export const USERNAME_MAX_LENGTH = 30; +export const USERNAME_MIN_LENGTH = 3; + +export const PASSWORD_MIN_LENGTH = 8; + +export const EMAIL_REGEX = new RegExp(/^\S+@\S+.\S+$/); + +export const URL_REGEX = new RegExp( + /^(http:\/\/www\.|https:\/\/www\.|http:\/\/|https:\/\/)[a-zA-Z0-9]+([\-\.]{1}[a-zA-Z0-9]+)*\.[a-zA-Z]{2,5}(:[0-9]{1,5})?(\/.*)?$/ +); + +/** + * ADDITIONAL_DETAILS_MAX_LENGTH defines the maximum length for the + * additionalDetails field on a flag. + */ +export const ADDITIONAL_DETAILS_MAX_LENGTH = 500; diff --git a/src/core/server/graph/tenant/mutators/Comment.ts b/src/core/server/graph/tenant/mutators/Comment.ts index c0a31a644..58fe34156 100644 --- a/src/core/server/graph/tenant/mutators/Comment.ts +++ b/src/core/server/graph/tenant/mutators/Comment.ts @@ -1,3 +1,4 @@ +import { ADDITIONAL_DETAILS_MAX_LENGTH } from "talk-common/helpers/validate"; import TenantContext from "talk-server/graph/tenant/context"; import { GQLCreateCommentDontAgreeInput, @@ -17,13 +18,8 @@ import { removeDontAgree, removeReaction, } from "talk-server/services/comments/actions"; -import { validateMaximumLength } from "./util"; -/** - * MAX_ADDITIONAL_DETAILS_LENGTH defines the maximum length for the - * additionalDetails field. - */ -const MAX_ADDITIONAL_DETAILS_LENGTH = 500; +import { validateMaximumLength } from "./util"; export const Comment = (ctx: TenantContext) => ({ create: ({ @@ -72,7 +68,7 @@ export const Comment = (ctx: TenantContext) => ({ commentRevisionID, // TODO: (wyattjoh) move this validation to the schema when bug is fixed: https://github.com/apollographql/graphql-tools/issues/842 additionalDetails: validateMaximumLength( - MAX_ADDITIONAL_DETAILS_LENGTH, + ADDITIONAL_DETAILS_MAX_LENGTH, additionalDetails ), }), @@ -92,7 +88,7 @@ export const Comment = (ctx: TenantContext) => ({ reason, // TODO: (wyattjoh) move this validation to the schema when bug is fixed: https://github.com/apollographql/graphql-tools/issues/842 additionalDetails: validateMaximumLength( - MAX_ADDITIONAL_DETAILS_LENGTH, + ADDITIONAL_DETAILS_MAX_LENGTH, additionalDetails ), }), diff --git a/src/core/server/models/user.ts b/src/core/server/models/user.ts index 5a0860629..5d80c28b3 100644 --- a/src/core/server/models/user.ts +++ b/src/core/server/models/user.ts @@ -145,8 +145,6 @@ export async function upsertUser( for (let profile of input.profiles) { switch (profile.type) { case "local": - // FIXME: (wyattjoh) add password validation here. - // Hash the user's password with bcrypt. const password = await hashPassword(profile.password); profile = { @@ -162,12 +160,8 @@ export async function upsertUser( // Add in the lowercase username if it was sent. if (input.username) { defaults.lowercaseUsername = input.username.toLowerCase(); - - // FIXME: (wyattjoh) add username checking regex here. } - // FIXME: (wyattjoh) add email validation here. - // Merge the defaults and the input together. const user: Readonly = { ...defaults, @@ -292,8 +286,6 @@ export async function updateUserPassword( id: string, password: string ) { - // FIXME: (wyattjoh) add password validation here. - // Hash the password. const hashedPassword = await hashPassword(password); @@ -359,8 +351,6 @@ export async function setUserUsername( // Lowercase the username. const lowercaseUsername = username.toLowerCase(); - // FIXME: (wyattjoh) add username checking regex here. - // Search to see if this username has been used before. let user = await collection(mongo).findOne({ tenantID, @@ -429,8 +419,6 @@ export async function setUserEmail( // Lowercase the email address. const email = emailAddress.toLowerCase(); - // FIXME: (wyattjoh) add email validation here. - // Search to see if this email has been used before. let user = await collection(mongo).findOne({ tenantID, @@ -500,9 +488,6 @@ export async function setUserLocalProfile( // Lowercase the email address. const email = emailAddress.toLowerCase(); - // FIXME: (wyattjoh) add email validation here. - // FIXME: (wyattjoh) add password validation here. - // Try to see if this local profile already exists on a User. let user = await retrieveUserWithProfile(mongo, tenantID, { type: "local", diff --git a/src/core/server/services/users/index.ts b/src/core/server/services/users/index.ts index c50ae8f1e..9393bdb19 100644 --- a/src/core/server/services/users/index.ts +++ b/src/core/server/services/users/index.ts @@ -1,7 +1,15 @@ import { Db } from "mongodb"; +import { + EMAIL_REGEX, + PASSWORD_MIN_LENGTH, + USERNAME_MAX_LENGTH, + USERNAME_MIN_LENGTH, + USERNAME_REGEX, +} from "talk-common/helpers/validate"; import { Tenant } from "talk-server/models/tenant"; import { + LocalProfile, setUserEmail, setUserLocalProfile, setUserUsername, @@ -11,14 +19,104 @@ import { User, } from "talk-server/models/user"; +/** + * validateUsername will validate that the username is valid. Current + * implementation uses a RegExp statically, future versions will expose this as + * configuration. + * + * @param tenant tenant where the User is associated with + * @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 + + if (!USERNAME_REGEX.test(username)) { + throw new Error("username contained illegal characters"); + } + + if (username.length > USERNAME_MAX_LENGTH) { + throw new Error("username exceeded maximum length"); + } + + if (username.length < USERNAME_MIN_LENGTH) { + throw new Error("username is too short"); + } +} + +/** + * validatePassword will validate that the password is valid. Current + * implementation uses a length statically, future versions will expose this as + * configuration. + * + * @param tenant tenant where the User is associated with + * @param password the password to be tested + */ +function validatePassword(tenant: Tenant, password: string) { + // TODO: replace these static length with database options in the Tenant eventually + if (password.length < PASSWORD_MIN_LENGTH) { + throw new Error("password is too short"); + } +} + +/** + * validateEmail will validate that the email is valid. Current implementation + * uses a length statically, future versions will expose this as configuration. + * + * @param tenant tenant where the User is associated with + * @param email the email to be tested + */ +function validateEmail(tenant: Tenant, email: string) { + // TODO: replace these static length with database options in the Tenant eventually + if (!EMAIL_REGEX.test(email)) { + throw new Error("email is in an invalid format"); + } +} + export type UpsertUser = UpsertUserInput; -export async function upsert(db: Db, tenant: Tenant, input: UpsertUser) { - const user = await upsertUser(db, tenant.id, input); +/** + * upsert 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) { + if (input.username) { + validateUsername(tenant, input.username); + } + + if (input.email) { + validateEmail(tenant, input.email); + } + + const localProfile: LocalProfile | undefined = input.profiles.find( + ({ type }) => type === "local" + ) as LocalProfile | undefined; + if (localProfile) { + validateEmail(tenant, localProfile.id); + validatePassword(tenant, localProfile.password); + + if (input.email !== localProfile.id) { + // TODO: (wyattjoh) return better error. + throw new Error("email addresses don't match profile"); + } + } + + const user = await upsertUser(mongo, tenant.id, input); return user; } +/** + * setUsername will set the username on the User if they don't already have one + * associated with them. + * + * @param mongo mongo database to interact with + * @param tenant Tenant where the User will be interacted with + * @param user User that should get their username changed + * @param username the new username for the User + */ export async function setUsername( mongo: Db, tenant: Tenant, @@ -30,9 +128,20 @@ export async function setUsername( throw new Error("username already associated with user"); } + validateUsername(tenant, username); + return setUserUsername(mongo, tenant.id, user.id, username); } +/** + * setEmail will set the email address on the User if they don't already have + * one associated with them. + * + * @param mongo mongo database to interact with + * @param tenant Tenant where the User will be interacted with + * @param user User that should get their username changed + * @param email the new email for the User + */ export async function setEmail( mongo: Db, tenant: Tenant, @@ -45,9 +154,23 @@ export async function setEmail( throw new Error("email address already associated with user"); } + validateEmail(tenant, email); + return setUserEmail(mongo, tenant.id, user.id, email); } +/** + * setPassword will set the password on the User if they don't already have + * one associated with them. This will allow the User to sign in with their + * current email address and new password if email based authentication is + * enabled. If the User does not have a email address associated with their + * account, this will fail. + * + * @param mongo mongo database to interact with + * @param tenant Tenant where the User will be interacted with + * @param user User that should get their password changed + * @param password the new password for the User + */ export async function setPassword( mongo: Db, tenant: Tenant, @@ -65,9 +188,22 @@ export async function setPassword( throw new Error("user already has local profile"); } + validatePassword(tenant, password); + return setUserLocalProfile(mongo, tenant.id, user.id, user.email, password); } +/** + * updatePassword will update the password associated with the User. If the User + * does not already have a password associated with their account, it will fail. + * If the User does not have an email address associated with the account, this + * will fail. + * + * @param mongo mongo database to interact with + * @param tenant Tenant where the User will be interacted with + * @param user User that should get their password changed + * @param password the new password for the User + */ export async function updatePassword( mongo: Db, tenant: Tenant, @@ -87,5 +223,7 @@ export async function updatePassword( throw new Error("user does not have a local profile"); } + validatePassword(tenant, password); + return updateUserPassword(mongo, tenant.id, user.id, password); }