Skip to content

Commit

Permalink
fix(site): username validation in forms (#1851)
Browse files Browse the repository at this point in the history
* refactor(site): move name validation to utils
* fix(site): username validation in forms
  • Loading branch information
oxy committed May 27, 2022
1 parent 8a5277e commit 14cdd85
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 59 deletions.
5 changes: 2 additions & 3 deletions site/src/components/CreateUserForm/CreateUserForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { FormikContextType, FormikErrors, useFormik } from "formik"
import React from "react"
import * as Yup from "yup"
import * as TypesGen from "../../api/typesGenerated"
import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils"
import { getFormHelpers, nameValidator, onChangeTrimmed } from "../../util/formUtils"
import { FormFooter } from "../FormFooter/FormFooter"
import { FullPageForm } from "../FullPageForm/FullPageForm"

Expand All @@ -15,7 +15,6 @@ export const Language = {
emailInvalid: "Please enter a valid email address.",
emailRequired: "Please enter an email address.",
passwordRequired: "Please enter a password.",
usernameRequired: "Please enter a username.",
createUser: "Create",
cancel: "Cancel",
}
Expand All @@ -32,7 +31,7 @@ export interface CreateUserFormProps {
const validationSchema = Yup.object({
email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired),
password: Yup.string().required(Language.passwordRequired),
username: Yup.string().required(Language.usernameRequired),
username: nameValidator(Language.usernameLabel),
})

export const CreateUserForm: React.FC<CreateUserFormProps> = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import TextField from "@material-ui/core/TextField"
import { FormikContextType, FormikErrors, useFormik } from "formik"
import React from "react"
import * as Yup from "yup"
import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils"
import { getFormHelpers, nameValidator, onChangeTrimmed } from "../../util/formUtils"
import { LoadingButton } from "../LoadingButton/LoadingButton"
import { Stack } from "../Stack/Stack"

Expand All @@ -22,7 +22,7 @@ export const Language = {

const validationSchema = Yup.object({
email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired),
username: Yup.string().trim(),
username: nameValidator(Language.usernameLabel),
})

export type AccountFormErrors = FormikErrors<AccountFormValues>
Expand Down
42 changes: 3 additions & 39 deletions site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { screen, waitFor } from "@testing-library/react"
import userEvent from "@testing-library/user-event"
import React from "react"
import { reach, StringSchema } from "yup"
import * as API from "../../api/api"
import { Language as FooterLanguage } from "../../components/FormFooter/FormFooter"
import { MockTemplate, MockWorkspace } from "../../testHelpers/entities"
import { renderWithAuth } from "../../testHelpers/renderHelpers"
import { Language as FormLanguage } from "../../util/formUtils"
import CreateWorkspacePage from "./CreateWorkspacePage"
import { Language, validationSchema } from "./CreateWorkspacePageView"
import { Language } from "./CreateWorkspacePageView"

const renderCreateWorkspacePage = () => {
return renderWithAuth(<CreateWorkspacePage />, {
Expand All @@ -23,8 +23,6 @@ const fillForm = async ({ name = "example" }: { name?: string }) => {
await userEvent.click(submitButton)
}

const nameSchema = reach(validationSchema, "name") as StringSchema

describe("CreateWorkspacePage", () => {
it("renders", async () => {
renderCreateWorkspacePage()
Expand All @@ -35,7 +33,7 @@ describe("CreateWorkspacePage", () => {
it("shows validation error message", async () => {
renderCreateWorkspacePage()
await fillForm({ name: "$$$" })
const errorMessage = await screen.findByText(Language.nameMatches)
const errorMessage = await screen.findByText(FormLanguage.nameInvalidChars(Language.nameLabel))
expect(errorMessage).toBeDefined()
})

Expand All @@ -47,38 +45,4 @@ describe("CreateWorkspacePage", () => {
// Check if the request was made
await waitFor(() => expect(API.createWorkspace).toBeCalledTimes(1))
})

describe("validationSchema", () => {
it("allows a 1-letter name", () => {
const validate = () => nameSchema.validateSync("t")
expect(validate).not.toThrow()
})

it("allows a 32-letter name", () => {
const input = Array(32).fill("a").join("")
const validate = () => nameSchema.validateSync(input)
expect(validate).not.toThrow()
})

it("allows 'test-3' to be used as name", () => {
const validate = () => nameSchema.validateSync("test-3")
expect(validate).not.toThrow()
})

it("allows '3-test' to be used as a name", () => {
const validate = () => nameSchema.validateSync("3-test")
expect(validate).not.toThrow()
})

it("disallows a 33-letter name", () => {
const input = Array(33).fill("a").join("")
const validate = () => nameSchema.validateSync(input)
expect(validate).toThrow()
})

it("disallows a space", () => {
const validate = () => nameSchema.validateSync("test 3")
expect(validate).toThrow()
})
})
})
16 changes: 2 additions & 14 deletions site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,13 @@ import { Loader } from "../../components/Loader/Loader"
import { Margins } from "../../components/Margins/Margins"
import { ParameterInput } from "../../components/ParameterInput/ParameterInput"
import { Stack } from "../../components/Stack/Stack"
import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils"
import { getFormHelpers, nameValidator, onChangeTrimmed } from "../../util/formUtils"

export const Language = {
templateLabel: "Template",
nameLabel: "Name",
nameRequired: "Please enter a name.",
nameMatches: "Name must start with a-Z or 0-9 and can contain a-Z, 0-9 or -",
nameMax: "Name cannot be longer than 32 characters",
}

// REMARK: Keep in sync with coderd/httpapi/httpapi.go#L40
const maxLenName = 32

// REMARK: Keep in sync with coderd/httpapi/httpapi.go#L18
const usernameRE = /^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$/

export interface CreateWorkspacePageViewProps {
loadingTemplates: boolean
loadingTemplateSchema: boolean
Expand All @@ -39,10 +30,7 @@ export interface CreateWorkspacePageViewProps {
}

export const validationSchema = Yup.object({
name: Yup.string()
.required(Language.nameRequired)
.matches(usernameRE, Language.nameMatches)
.max(maxLenName, Language.nameMax),
name: nameValidator(Language.nameLabel),
})

export const CreateWorkspacePageView: React.FC<CreateWorkspacePageViewProps> = (props) => {
Expand Down
38 changes: 37 additions & 1 deletion site/src/util/formUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { FormikContextType } from "formik/dist/types"
import { getFormHelpers, onChangeTrimmed } from "./formUtils"
import { getFormHelpers, nameValidator, onChangeTrimmed } from "./formUtils"

interface TestType {
untouchedGoodField: string
Expand Down Expand Up @@ -35,6 +35,8 @@ const form = {
},
} as unknown as FormikContextType<TestType>

const nameSchema = nameValidator("name")

describe("form util functions", () => {
describe("getFormHelpers", () => {
describe("without API errors", () => {
Expand Down Expand Up @@ -94,4 +96,38 @@ describe("form util functions", () => {
expect(mockHandleChange).toHaveBeenCalledWith({ target: { value: "hello" } })
})
})

describe("nameValidator", () => {
it("allows a 1-letter name", () => {
const validate = () => nameSchema.validateSync("a")
expect(validate).not.toThrow()
})

it("allows a 32-letter name", () => {
const input = Array(32).fill("a").join("")
const validate = () => nameSchema.validateSync(input)
expect(validate).not.toThrow()
})

it("allows 'test-3' to be used as name", () => {
const validate = () => nameSchema.validateSync("test-3")
expect(validate).not.toThrow()
})

it("allows '3-test' to be used as a name", () => {
const validate = () => nameSchema.validateSync("3-test")
expect(validate).not.toThrow()
})

it("disallows a 33-letter name", () => {
const input = Array(33).fill("a").join("")
const validate = () => nameSchema.validateSync(input)
expect(validate).toThrow()
})

it("disallows a space", () => {
const validate = () => nameSchema.validateSync("test 3")
expect(validate).toThrow()
})
})
})
26 changes: 26 additions & 0 deletions site/src/util/formUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
import { FormikContextType, FormikErrors, getIn } from "formik"
import { ChangeEvent, ChangeEventHandler, FocusEventHandler, ReactNode } from "react"
import * as Yup from "yup"

export const Language = {
nameRequired: (name: string): string => {
return `Please enter a ${name.toLowerCase()}.`
},
nameInvalidChars: (name: string): string => {
return `${name} must start with a-Z or 0-9 and can contain a-Z, 0-9 or -`
},
nameTooLong: (name: string): string => {
return `${name} cannot be longer than 32 characters`
},
}

interface FormHelpers {
name: string
Expand Down Expand Up @@ -38,3 +51,16 @@ export const onChangeTrimmed =
event.target.value = event.target.value.trim()
form.handleChange(event)
}

// REMARK: Keep in sync with coderd/httpapi/httpapi.go#L40
const maxLenName = 32

// REMARK: Keep in sync with coderd/httpapi/httpapi.go#L18
const usernameRE = /^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$/

// REMARK: see #1756 for name/username semantics
export const nameValidator = (name: string): Yup.StringSchema =>
Yup.string()
.required(Language.nameRequired(name))
.matches(usernameRE, Language.nameInvalidChars(name))
.max(maxLenName, Language.nameTooLong(name))

0 comments on commit 14cdd85

Please sign in to comment.