fix(site): username validation in forms (#1851)

* refactor(site): move name validation to utils
* fix(site): username validation in forms
This commit is contained in:
Oxylibrium
2022-05-27 13:02:56 -04:00
committed by GitHub
parent 8a5277e291
commit 14cdd85b66
6 changed files with 72 additions and 59 deletions

View File

@ -4,7 +4,7 @@ import { FormikContextType, FormikErrors, useFormik } from "formik"
import React from "react" import React from "react"
import * as Yup from "yup" import * as Yup from "yup"
import * as TypesGen from "../../api/typesGenerated" 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 { FormFooter } from "../FormFooter/FormFooter"
import { FullPageForm } from "../FullPageForm/FullPageForm" import { FullPageForm } from "../FullPageForm/FullPageForm"
@ -15,7 +15,6 @@ export const Language = {
emailInvalid: "Please enter a valid email address.", emailInvalid: "Please enter a valid email address.",
emailRequired: "Please enter an email address.", emailRequired: "Please enter an email address.",
passwordRequired: "Please enter a password.", passwordRequired: "Please enter a password.",
usernameRequired: "Please enter a username.",
createUser: "Create", createUser: "Create",
cancel: "Cancel", cancel: "Cancel",
} }
@ -32,7 +31,7 @@ export interface CreateUserFormProps {
const validationSchema = Yup.object({ const validationSchema = Yup.object({
email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired), email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired),
password: Yup.string().required(Language.passwordRequired), password: Yup.string().required(Language.passwordRequired),
username: Yup.string().required(Language.usernameRequired), username: nameValidator(Language.usernameLabel),
}) })
export const CreateUserForm: React.FC<CreateUserFormProps> = ({ export const CreateUserForm: React.FC<CreateUserFormProps> = ({

View File

@ -3,7 +3,7 @@ import TextField from "@material-ui/core/TextField"
import { FormikContextType, FormikErrors, useFormik } from "formik" import { FormikContextType, FormikErrors, useFormik } from "formik"
import React from "react" import React from "react"
import * as Yup from "yup" import * as Yup from "yup"
import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils" import { getFormHelpers, nameValidator, onChangeTrimmed } from "../../util/formUtils"
import { LoadingButton } from "../LoadingButton/LoadingButton" import { LoadingButton } from "../LoadingButton/LoadingButton"
import { Stack } from "../Stack/Stack" import { Stack } from "../Stack/Stack"
@ -22,7 +22,7 @@ export const Language = {
const validationSchema = Yup.object({ const validationSchema = Yup.object({
email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired), email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired),
username: Yup.string().trim(), username: nameValidator(Language.usernameLabel),
}) })
export type AccountFormErrors = FormikErrors<AccountFormValues> export type AccountFormErrors = FormikErrors<AccountFormValues>

View File

@ -1,13 +1,13 @@
import { screen, waitFor } from "@testing-library/react" import { screen, waitFor } from "@testing-library/react"
import userEvent from "@testing-library/user-event" import userEvent from "@testing-library/user-event"
import React from "react" import React from "react"
import { reach, StringSchema } from "yup"
import * as API from "../../api/api" import * as API from "../../api/api"
import { Language as FooterLanguage } from "../../components/FormFooter/FormFooter" import { Language as FooterLanguage } from "../../components/FormFooter/FormFooter"
import { MockTemplate, MockWorkspace } from "../../testHelpers/entities" import { MockTemplate, MockWorkspace } from "../../testHelpers/entities"
import { renderWithAuth } from "../../testHelpers/renderHelpers" import { renderWithAuth } from "../../testHelpers/renderHelpers"
import { Language as FormLanguage } from "../../util/formUtils"
import CreateWorkspacePage from "./CreateWorkspacePage" import CreateWorkspacePage from "./CreateWorkspacePage"
import { Language, validationSchema } from "./CreateWorkspacePageView" import { Language } from "./CreateWorkspacePageView"
const renderCreateWorkspacePage = () => { const renderCreateWorkspacePage = () => {
return renderWithAuth(<CreateWorkspacePage />, { return renderWithAuth(<CreateWorkspacePage />, {
@ -23,8 +23,6 @@ const fillForm = async ({ name = "example" }: { name?: string }) => {
await userEvent.click(submitButton) await userEvent.click(submitButton)
} }
const nameSchema = reach(validationSchema, "name") as StringSchema
describe("CreateWorkspacePage", () => { describe("CreateWorkspacePage", () => {
it("renders", async () => { it("renders", async () => {
renderCreateWorkspacePage() renderCreateWorkspacePage()
@ -35,7 +33,7 @@ describe("CreateWorkspacePage", () => {
it("shows validation error message", async () => { it("shows validation error message", async () => {
renderCreateWorkspacePage() renderCreateWorkspacePage()
await fillForm({ name: "$$$" }) await fillForm({ name: "$$$" })
const errorMessage = await screen.findByText(Language.nameMatches) const errorMessage = await screen.findByText(FormLanguage.nameInvalidChars(Language.nameLabel))
expect(errorMessage).toBeDefined() expect(errorMessage).toBeDefined()
}) })
@ -47,38 +45,4 @@ describe("CreateWorkspacePage", () => {
// Check if the request was made // Check if the request was made
await waitFor(() => expect(API.createWorkspace).toBeCalledTimes(1)) 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()
})
})
}) })

View File

@ -10,22 +10,13 @@ import { Loader } from "../../components/Loader/Loader"
import { Margins } from "../../components/Margins/Margins" import { Margins } from "../../components/Margins/Margins"
import { ParameterInput } from "../../components/ParameterInput/ParameterInput" import { ParameterInput } from "../../components/ParameterInput/ParameterInput"
import { Stack } from "../../components/Stack/Stack" import { Stack } from "../../components/Stack/Stack"
import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils" import { getFormHelpers, nameValidator, onChangeTrimmed } from "../../util/formUtils"
export const Language = { export const Language = {
templateLabel: "Template", templateLabel: "Template",
nameLabel: "Name", 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 { export interface CreateWorkspacePageViewProps {
loadingTemplates: boolean loadingTemplates: boolean
loadingTemplateSchema: boolean loadingTemplateSchema: boolean
@ -39,10 +30,7 @@ export interface CreateWorkspacePageViewProps {
} }
export const validationSchema = Yup.object({ export const validationSchema = Yup.object({
name: Yup.string() name: nameValidator(Language.nameLabel),
.required(Language.nameRequired)
.matches(usernameRE, Language.nameMatches)
.max(maxLenName, Language.nameMax),
}) })
export const CreateWorkspacePageView: React.FC<CreateWorkspacePageViewProps> = (props) => { export const CreateWorkspacePageView: React.FC<CreateWorkspacePageViewProps> = (props) => {

View File

@ -1,5 +1,5 @@
import { FormikContextType } from "formik/dist/types" import { FormikContextType } from "formik/dist/types"
import { getFormHelpers, onChangeTrimmed } from "./formUtils" import { getFormHelpers, nameValidator, onChangeTrimmed } from "./formUtils"
interface TestType { interface TestType {
untouchedGoodField: string untouchedGoodField: string
@ -35,6 +35,8 @@ const form = {
}, },
} as unknown as FormikContextType<TestType> } as unknown as FormikContextType<TestType>
const nameSchema = nameValidator("name")
describe("form util functions", () => { describe("form util functions", () => {
describe("getFormHelpers", () => { describe("getFormHelpers", () => {
describe("without API errors", () => { describe("without API errors", () => {
@ -94,4 +96,38 @@ describe("form util functions", () => {
expect(mockHandleChange).toHaveBeenCalledWith({ target: { value: "hello" } }) 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()
})
})
}) })

View File

@ -1,5 +1,18 @@
import { FormikContextType, FormikErrors, getIn } from "formik" import { FormikContextType, FormikErrors, getIn } from "formik"
import { ChangeEvent, ChangeEventHandler, FocusEventHandler, ReactNode } from "react" 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 { interface FormHelpers {
name: string name: string
@ -38,3 +51,16 @@ export const onChangeTrimmed =
event.target.value = event.target.value.trim() event.target.value = event.target.value.trim()
form.handleChange(event) 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))