[next] User Input Validation (#2125)

* feat: added user validation

* fix: adapt client validation to match servers

* fix: snapshots

* feat: moved validation code to common

* chore: renamed utils/validate to helpers/validate

src/core/client/auth/test/__snapshots__/signUp.spec.tsx.snap

@@ -1201,7 +1201,7 @@ exports[`checks for too long username 1`] = `
               warning
             </span>
             <span>
-              Usernames cannot be longer than ⁨20⁩ characters.
+              Usernames cannot be longer than ⁨30⁩ characters.
             </span>
           </div>
         </div>

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)
 );
 
 /**

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;

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
       ),
     }),

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<User> = {
     ...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",

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);
 }