-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[VPAT-55] chore(security): implement input validation across authentication and workspace forms #8528
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
base: preview
Are you sure you want to change the base?
[VPAT-55] chore(security): implement input validation across authentication and workspace forms #8528
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import { TOAST_TYPE, setToast } from "@plane/propel/toast"; | |
| import type { IUser, IWorkspace, TOnboardingSteps } from "@plane/types"; | ||
| // ui | ||
| import { CustomSelect, Input, Spinner } from "@plane/ui"; | ||
| import { validateWorkspaceName, validateSlug } from "@plane/utils"; | ||
| // hooks | ||
| import { useWorkspace } from "@/hooks/store/use-workspace"; | ||
| import { useUserProfile, useUserSettings } from "@/hooks/store/user"; | ||
|
|
@@ -132,8 +133,7 @@ export const CreateWorkspace = observer(function CreateWorkspace(props: Props) { | |
| name="name" | ||
| rules={{ | ||
| required: t("common.errors.required"), | ||
| validate: (value) => | ||
| /^[\w\s-]*$/.test(value) || t("workspace_creation.errors.validation.name_alphanumeric"), | ||
| validate: (value) => validateWorkspaceName(value, true), | ||
| maxLength: { | ||
| value: 80, | ||
| message: t("workspace_creation.errors.validation.name_length"), | ||
|
|
@@ -194,7 +194,8 @@ export const CreateWorkspace = observer(function CreateWorkspace(props: Props) { | |
| type="text" | ||
| value={value.toLocaleLowerCase().trim().replace(/ /g, "-")} | ||
| onChange={(e) => { | ||
| if (/^[a-zA-Z0-9_-]+$/.test(e.target.value)) setInvalidSlug(false); | ||
| const validation = validateSlug(e.target.value); | ||
| if (validation === true) setInvalidSlug(false); | ||
| else setInvalidSlug(true); | ||
| onChange(e.target.value.toLowerCase()); | ||
|
Comment on lines
196
to
200
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same slug validation issue as in The slug validation runs on the raw input ( onChange={(e) => {
- const validation = validateSlug(e.target.value);
+ const transformedSlug = e.target.value.toLowerCase().trim().replace(/ /g, "-");
+ const validation = validateSlug(transformedSlug);
if (validation === true) setInvalidSlug(false);
else setInvalidSlug(true);
onChange(e.target.value.toLowerCase());
}}🤖 Prompt for AI Agents |
||
| }} | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,7 +9,7 @@ import { Button } from "@plane/propel/button"; | |||||||||||||||||||||||||||
| import { TOAST_TYPE, setToast } from "@plane/propel/toast"; | ||||||||||||||||||||||||||||
| import type { IUser, IWorkspace } from "@plane/types"; | ||||||||||||||||||||||||||||
| import { Spinner } from "@plane/ui"; | ||||||||||||||||||||||||||||
| import { cn } from "@plane/utils"; | ||||||||||||||||||||||||||||
| import { cn, validateWorkspaceName, validateSlug } from "@plane/utils"; | ||||||||||||||||||||||||||||
| // hooks | ||||||||||||||||||||||||||||
| import { useWorkspace } from "@/hooks/store/use-workspace"; | ||||||||||||||||||||||||||||
| import { useUserProfile, useUserSettings } from "@/hooks/store/user"; | ||||||||||||||||||||||||||||
|
|
@@ -139,8 +139,7 @@ export const WorkspaceCreateStep = observer(function WorkspaceCreateStep({ | |||||||||||||||||||||||||||
| name="name" | ||||||||||||||||||||||||||||
| rules={{ | ||||||||||||||||||||||||||||
| required: t("common.errors.required"), | ||||||||||||||||||||||||||||
| validate: (value) => | ||||||||||||||||||||||||||||
| /^[\w\s-]*$/.test(value) || t("workspace_creation.errors.validation.name_alphanumeric"), | ||||||||||||||||||||||||||||
| validate: (value) => validateWorkspaceName(value, true), | ||||||||||||||||||||||||||||
| maxLength: { | ||||||||||||||||||||||||||||
| value: 80, | ||||||||||||||||||||||||||||
| message: t("workspace_creation.errors.validation.name_length"), | ||||||||||||||||||||||||||||
|
|
@@ -213,7 +212,8 @@ export const WorkspaceCreateStep = observer(function WorkspaceCreateStep({ | |||||||||||||||||||||||||||
| type="text" | ||||||||||||||||||||||||||||
| value={value.toLocaleLowerCase().trim().replace(/ /g, "-")} | ||||||||||||||||||||||||||||
| onChange={(e) => { | ||||||||||||||||||||||||||||
| if (/^[a-zA-Z0-9_-]+$/.test(e.target.value)) setInvalidSlug(false); | ||||||||||||||||||||||||||||
| const validation = validateSlug(e.target.value); | ||||||||||||||||||||||||||||
| if (validation === true) setInvalidSlug(false); | ||||||||||||||||||||||||||||
| else setInvalidSlug(true); | ||||||||||||||||||||||||||||
| onChange(e.target.value.toLowerCase()); | ||||||||||||||||||||||||||||
|
Comment on lines
214
to
218
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Slug validation runs on raw input before transformation. The displayed slug value applies transformations (lowercase, trim, replace spaces with hyphens on Line 213), but Consider validating the transformed value instead: onChange={(e) => {
- const validation = validateSlug(e.target.value);
+ const transformedSlug = e.target.value.toLowerCase().trim().replace(/ /g, "-");
+ const validation = validateSlug(transformedSlug);
if (validation === true) setInvalidSlug(false);
else setInvalidSlug(true);
onChange(e.target.value.toLowerCase());
}}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| }} | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,8 @@ import { handleCoverImageChange } from "@/helpers/cover-image.helper"; | |
| // hooks | ||
| import { useInstance } from "@/hooks/store/use-instance"; | ||
| import { useUser, useUserProfile } from "@/hooks/store/user"; | ||
| // utils | ||
| import { validatePersonName, validateDisplayName } from "@plane/utils"; | ||
|
|
||
| type TUserProfileForm = { | ||
| avatar_url: string; | ||
|
|
@@ -255,6 +257,7 @@ export const ProfileForm = observer(function ProfileForm(props: TProfileFormProp | |
| name="first_name" | ||
| rules={{ | ||
| required: "Please enter first name", | ||
| validate: validatePersonName, | ||
| }} | ||
| render={({ field: { value, onChange, ref } }) => ( | ||
| <Input | ||
|
|
@@ -267,7 +270,7 @@ export const ProfileForm = observer(function ProfileForm(props: TProfileFormProp | |
| hasError={Boolean(errors.first_name)} | ||
| placeholder="Enter your first name" | ||
| className={`w-full rounded-md ${errors.first_name ? "border-danger-strong" : ""}`} | ||
| maxLength={24} | ||
| maxLength={50} | ||
| autoComplete="on" | ||
| /> | ||
| )} | ||
|
|
@@ -279,6 +282,9 @@ export const ProfileForm = observer(function ProfileForm(props: TProfileFormProp | |
| <Controller | ||
| control={control} | ||
| name="last_name" | ||
| rules={{ | ||
| validate: validatePersonName, | ||
| }} | ||
|
Comment on lines
+285
to
+287
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validation mismatch: last_name appears optional but validation requires it. The Either mark the field as required in the UI, or use a validator that permits empty values: 🔧 Option 1: Create an optional person name validatorIn export const validateOptionalPersonName = (name: string): boolean | string => {
if (!name || name.trim() === "") {
return true; // Allow empty
}
// Rest of validation...
if (name.length > 50) {
return "Name must be 50 characters or less";
}
if (!PERSON_NAME_REGEX.test(name)) {
return "Names can only contain letters, spaces, hyphens, and apostrophes";
}
return true;
};Then use it here: rules={{
- validate: validatePersonName,
+ validate: validateOptionalPersonName,
}}🔧 Option 2: Mark last_name as required in UI-<h4 className="text-13 font-medium text-secondary">{t("last_name")}</h4>
+<h4 className="text-13 font-medium text-secondary">
+ {t("last_name")}
+ <span className="text-danger-primary">*</span>
+</h4>🤖 Prompt for AI Agents |
||
| render={({ field: { value, onChange, ref } }) => ( | ||
| <Input | ||
| id="last_name" | ||
|
|
@@ -290,11 +296,12 @@ export const ProfileForm = observer(function ProfileForm(props: TProfileFormProp | |
| hasError={Boolean(errors.last_name)} | ||
| placeholder="Enter your last name" | ||
| className="w-full rounded-md" | ||
| maxLength={24} | ||
| maxLength={50} | ||
| autoComplete="on" | ||
| /> | ||
| )} | ||
| /> | ||
| {errors.last_name && <span className="text-11 text-danger-primary">{errors.last_name.message}</span>} | ||
| </div> | ||
| <div className="flex flex-col gap-1"> | ||
| <h4 className="text-13 font-medium text-secondary"> | ||
|
|
@@ -306,14 +313,7 @@ export const ProfileForm = observer(function ProfileForm(props: TProfileFormProp | |
| name="display_name" | ||
| rules={{ | ||
| required: "Display name is required.", | ||
| validate: (value) => { | ||
| if (value.trim().length < 1) return "Display name can't be empty."; | ||
| if (value.split(" ").length > 1) return "Display name can't have two consecutive spaces."; | ||
| if (value.replace(/\s/g, "").length < 1) return "Display name must be at least 1 character long."; | ||
| if (value.replace(/\s/g, "").length > 20) | ||
| return "Display name must be less than 20 characters long."; | ||
| return true; | ||
| }, | ||
| validate: validateDisplayName, | ||
| }} | ||
| render={({ field: { value, onChange, ref } }) => ( | ||
| <Input | ||
|
|
@@ -326,7 +326,7 @@ export const ProfileForm = observer(function ProfileForm(props: TProfileFormProp | |
| hasError={Boolean(errors?.display_name)} | ||
| placeholder="Enter your display name" | ||
| className={`w-full ${errors?.display_name ? "border-danger-strong" : ""}`} | ||
| maxLength={24} | ||
| maxLength={50} | ||
| /> | ||
| )} | ||
| /> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Company name
requiredparameter inconsistent with UI.The UI marks company name as required (Line 228 shows the asterisk), but
validateCompanyNameis called withrequired: false. If the field is truly required, passtrueto enforce validation:🤖 Prompt for AI Agents