Add suggested PR review improvements, better validation on ssh cert template modal

This commit is contained in:
Tuan Dang
2024-12-10 11:34:08 -08:00
parent 48174e2500
commit c5816014a6
6 changed files with 119 additions and 52 deletions
@@ -61,36 +61,41 @@ export const registerSshCertificateTemplateRouter = async (server: FastifyZodPro
rateLimit: writeLimit rateLimit: writeLimit
}, },
schema: { schema: {
body: z.object({ body: z
sshCaId: z.string().describe(SSH_CERTIFICATE_TEMPLATES.CREATE.sshCaId), .object({
name: z sshCaId: z.string().describe(SSH_CERTIFICATE_TEMPLATES.CREATE.sshCaId),
.string() name: z
.min(1) .string()
.max(36) .min(1)
.refine((v) => slugify(v) === v, { .max(36)
message: "Name must be a valid slug" .refine((v) => slugify(v) === v, {
}) message: "Name must be a valid slug"
.describe(SSH_CERTIFICATE_TEMPLATES.CREATE.name), })
ttl: z .describe(SSH_CERTIFICATE_TEMPLATES.CREATE.name),
.string() ttl: z
.refine((val) => ms(val) > 0, "TTL must be a positive number") .string()
.default("1h") .refine((val) => ms(val) > 0, "TTL must be a positive number")
.describe(SSH_CERTIFICATE_TEMPLATES.CREATE.ttl), .default("1h")
maxTTL: z .describe(SSH_CERTIFICATE_TEMPLATES.CREATE.ttl),
.string() maxTTL: z
.refine((val) => ms(val) > 0, "Max TTL must be a positive number") .string()
.default("30d") .refine((val) => ms(val) > 0, "Max TTL must be a positive number")
.describe(SSH_CERTIFICATE_TEMPLATES.CREATE.maxTTL), .default("30d")
allowedUsers: z .describe(SSH_CERTIFICATE_TEMPLATES.CREATE.maxTTL),
.array(z.string().refine(isValidUserPattern, "Invalid user pattern")) allowedUsers: z
.describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowedUsers), .array(z.string().refine(isValidUserPattern, "Invalid user pattern"))
allowedHosts: z .describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowedUsers),
.array(z.string().refine(isValidHostPattern, "Invalid host pattern")) allowedHosts: z
.describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowedHosts), .array(z.string().refine(isValidHostPattern, "Invalid host pattern"))
allowUserCertificates: z.boolean().describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowUserCertificates), .describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowedHosts),
allowHostCertificates: z.boolean().describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowHostCertificates), allowUserCertificates: z.boolean().describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowUserCertificates),
allowCustomKeyIds: z.boolean().describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowCustomKeyIds) allowHostCertificates: z.boolean().describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowHostCertificates),
}), allowCustomKeyIds: z.boolean().describe(SSH_CERTIFICATE_TEMPLATES.CREATE.allowCustomKeyIds)
})
.refine((data) => ms(data.maxTTL) > ms(data.ttl), {
message: "Max TLL must be greater than TTL",
path: ["maxTTL"]
}),
response: { response: {
200: sanitizedSshCertificateTemplate 200: sanitizedSshCertificateTemplate
} }
@@ -905,7 +905,7 @@ export const projectServiceFactory = ({
}; };
/** /**
* Return list of SSH certificates for organization * Return list of SSH certificates for project
*/ */
const listProjectSshCertificates = async ({ const listProjectSshCertificates = async ({
limit = 25, limit = 25,
@@ -945,7 +945,7 @@ export const projectServiceFactory = ({
}; };
/** /**
* Return list of SSH certificate templates for organization * Return list of SSH certificate templates for project
*/ */
const listProjectSshCertificateTemplates = async ({ const listProjectSshCertificateTemplates = async ({
actorId, actorId,
@@ -75,7 +75,6 @@ export const SshCertificateModal = ({ popUp, handlePopUpToggle }: Props) => {
const popUpData = popUp?.sshCertificate?.data as { const popUpData = popUp?.sshCertificate?.data as {
sshCaId: string; sshCaId: string;
templateName: string;
templateId: string; templateId: string;
}; };
@@ -1,6 +1,8 @@
import { useEffect } from "react"; import { useEffect } from "react";
import { Controller, useForm } from "react-hook-form"; import { Controller, useForm } from "react-hook-form";
import { zodResolver } from "@hookform/resolvers/zod"; import { zodResolver } from "@hookform/resolvers/zod";
import slugify from "@sindresorhus/slugify";
import ms from "ms";
import { z } from "zod"; import { z } from "zod";
import { createNotification } from "@app/components/notifications"; import { createNotification } from "@app/components/notifications";
@@ -24,17 +26,79 @@ import {
} from "@app/hooks/api"; } from "@app/hooks/api";
import { UsePopUpState } from "@app/hooks/usePopUp"; import { UsePopUpState } from "@app/hooks/usePopUp";
const schema = z.object({ // Validates usernames or wildcard (*)
sshCaId: z.string(), export const isValidUserPattern = (value: string): boolean => {
name: z.string().min(1), // Matches valid Linux usernames or a wildcard (*)
ttl: z.string().trim().min(1), const userRegex = /^(?:\*|[a-z_][a-z0-9_-]{0,31})$/;
maxTTL: z.string().trim().min(1), return userRegex.test(value);
allowedUsers: z.string(), };
allowedHosts: z.string(),
allowUserCertificates: z.boolean().optional().default(false), // Validates hostnames, wildcard domains, or IP addresses
allowHostCertificates: z.boolean().optional().default(false), export const isValidHostPattern = (value: string): boolean => {
allowCustomKeyIds: z.boolean().optional().default(false) // Matches FQDNs, wildcard domains (*.example.com), IPv4, and IPv6 addresses
}); const hostRegex =
/^(?:\*|\*\.[a-z0-9-]+(?:\.[a-z0-9-]+)*|[a-z0-9-]+(?:\.[a-z0-9-]+)*|\d{1,3}(\.\d{1,3}){3}|([a-fA-F0-9:]+:+)+[a-fA-F0-9]+(?:%[a-zA-Z0-9]+)?)$/;
return hostRegex.test(value);
};
const schema = z
.object({
sshCaId: z.string(),
name: z
.string()
.trim()
.toLowerCase()
.min(1)
.max(36)
.refine((v) => slugify(v) === v, {
message: "Invalid name. Name can only contain alphanumeric characters and hyphens."
}),
ttl: z
.string()
.trim()
.refine(
(val) => ms(val) > 0,
"TTL must be a valid time string such as 2 days, 1d, 2h 1y, ..."
)
.default("1h"),
maxTTL: z
.string()
.trim()
.refine(
(val) => ms(val) > 0,
"Max TTL must be a valid time string such as 2 days, 1d, 2h 1y, ..."
)
.default("30d"),
allowedUsers: z.string().refine(
(val) => {
const trimmed = val.trim();
if (trimmed === "") return true;
const users = trimmed.split(",").map((u) => u.trim());
return users.every(isValidUserPattern);
},
{
message: "Invalid user pattern in allowedUsers"
}
),
allowedHosts: z.string().refine(
(val) => {
const trimmed = val.trim();
if (trimmed === "") return true;
const users = trimmed.split(",").map((u) => u.trim());
return users.every(isValidHostPattern);
},
{
message: "Invalid host pattern in allowedHosts"
}
),
allowUserCertificates: z.boolean().optional().default(false),
allowHostCertificates: z.boolean().optional().default(false),
allowCustomKeyIds: z.boolean().optional().default(false)
})
.refine((data) => ms(data.maxTTL) > ms(data.ttl), {
message: "Max TLL must be greater than TTL",
path: ["maxTTL"]
});
export type FormData = z.infer<typeof schema>; export type FormData = z.infer<typeof schema>;
@@ -54,14 +54,14 @@ export const SshCertificateTemplatesSection = ({ caId }: Props) => {
}; };
const onUpdateSshCaStatus = async ({ const onUpdateSshCaStatus = async ({
certTemplateId, templateId,
status status
}: { }: {
certTemplateId: string; templateId: string;
status: SshCertTemplateStatus; status: SshCertTemplateStatus;
}) => { }) => {
try { try {
await updateSshCertTemplate({ id: certTemplateId, status }); await updateSshCertTemplate({ id: templateId, status });
await createNotification({ await createNotification({
text: `Successfully ${ text: `Successfully ${
@@ -144,7 +144,7 @@ export const SshCertificateTemplatesSection = ({ caId }: Props) => {
onDeleteApproved={() => onDeleteApproved={() =>
onUpdateSshCaStatus( onUpdateSshCaStatus(
popUp?.sshCertificateTemplateStatus?.data as { popUp?.sshCertificateTemplateStatus?.data as {
certTemplateId: string; templateId: string;
status: SshCertTemplateStatus; status: SshCertTemplateStatus;
} }
) )
@@ -47,9 +47,8 @@ type Props = {
id?: string; id?: string;
name?: string; name?: string;
sshCaId?: string; sshCaId?: string;
certTemplateId?: string;
status?: SshCertTemplateStatus; status?: SshCertTemplateStatus;
templateName?: string; templateId?: string;
} }
) => void; ) => void;
}; };
@@ -101,7 +100,7 @@ export const SshCertificateTemplatesTable = ({ handlePopUpOpen, sshCaId }: Props
onClick={(e) => { onClick={(e) => {
e.stopPropagation(); e.stopPropagation();
handlePopUpOpen("sshCertificateTemplateStatus", { handlePopUpOpen("sshCertificateTemplateStatus", {
certTemplateId: certificateTemplate.id, templateId: certificateTemplate.id,
status: status:
certificateTemplate.status === SshCertTemplateStatus.ACTIVE certificateTemplate.status === SshCertTemplateStatus.ACTIVE
? SshCertTemplateStatus.DISABLED ? SshCertTemplateStatus.DISABLED
@@ -127,7 +126,7 @@ export const SshCertificateTemplatesTable = ({ handlePopUpOpen, sshCaId }: Props
onClick={() => { onClick={() => {
handlePopUpOpen("sshCertificate", { handlePopUpOpen("sshCertificate", {
sshCaId, sshCaId,
templateName: certificateTemplate.name templateId: certificateTemplate.id
}); });
}} }}
icon={ icon={