From 00039ba0e4a4e0e69d43b0580c3f30eab9598a12 Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Mon, 19 Aug 2024 16:15:43 +0800 Subject: [PATCH] misc: addressed PR feedback regarding audit logs and endpoint structure --- .../ee/services/audit-log/audit-log-types.ts | 12 ++++ .../routes/v1/certificate-template-router.ts | 57 +++++++---------- .../src/server/routes/v2/project-router.ts | 14 +--- .../certificate-template-dal.ts | 64 +++++++++++-------- .../certificate-template-schema.ts | 18 ++++++ .../certificate-template-service.ts | 26 ++++++-- frontend/src/hooks/api/auditLogs/types.tsx | 12 ++++ .../api/certificateTemplates/mutations.tsx | 8 +-- .../api/certificateTemplates/queries.tsx | 4 +- .../hooks/api/certificateTemplates/types.ts | 9 +-- frontend/src/hooks/api/workspace/queries.tsx | 4 +- .../AuditLogsPage/components/LogsTableRow.tsx | 13 ++++ 12 files changed, 145 insertions(+), 96 deletions(-) create mode 100644 backend/src/services/certificate-template/certificate-template-schema.ts diff --git a/backend/src/ee/services/audit-log/audit-log-types.ts b/backend/src/ee/services/audit-log/audit-log-types.ts index 50549417d..0d07d005b 100644 --- a/backend/src/ee/services/audit-log/audit-log-types.ts +++ b/backend/src/ee/services/audit-log/audit-log-types.ts @@ -1374,6 +1374,12 @@ interface CreateCertificateTemplate { type: EventType.CREATE_CERTIFICATE_TEMPLATE; metadata: { certificateTemplateId: string; + caId: string; + pkiCollectionId?: string; + name: string; + commonName: string; + subjectAlternativeName: string; + ttl: string; }; } @@ -1388,6 +1394,12 @@ interface UpdateCertificateTemplate { type: EventType.UPDATE_CERTIFICATE_TEMPLATE; metadata: { certificateTemplateId: string; + caId: string; + pkiCollectionId?: string; + name: string; + commonName: string; + subjectAlternativeName: string; + ttl: string; }; } diff --git a/backend/src/server/routes/v1/certificate-template-router.ts b/backend/src/server/routes/v1/certificate-template-router.ts index 861279094..3f3ecd5b3 100644 --- a/backend/src/server/routes/v1/certificate-template-router.ts +++ b/backend/src/server/routes/v1/certificate-template-router.ts @@ -1,24 +1,14 @@ import ms from "ms"; import { z } from "zod"; -import { CertificateTemplatesSchema } from "@app/db/schemas"; import { EventType } from "@app/ee/services/audit-log/audit-log-types"; import { CERTIFICATE_TEMPLATES } from "@app/lib/api-docs"; import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { AuthMode } from "@app/services/auth/auth-type"; +import { sanitizedCertificateTemplate } from "@app/services/certificate-template/certificate-template-schema"; import { validateTemplateRegexField } from "@app/services/certificate-template/certificate-template-validators"; -const sanitizedCertificateTemplate = CertificateTemplatesSchema.pick({ - id: true, - caId: true, - name: true, - commonName: true, - subjectAlternativeName: true, - pkiCollectionId: true, - ttl: true -}); - export const registerCertificateTemplateRouter = async (server: FastifyZodProvider) => { server.route({ method: "GET", @@ -31,14 +21,7 @@ export const registerCertificateTemplateRouter = async (server: FastifyZodProvid certificateTemplateId: z.string().describe(CERTIFICATE_TEMPLATES.GET.certificateTemplateId) }), response: { - 200: z.object({ - certificateTemplate: sanitizedCertificateTemplate.merge( - z.object({ - projectId: z.string(), - caName: z.string() - }) - ) - }) + 200: sanitizedCertificateTemplate } }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), @@ -62,7 +45,7 @@ export const registerCertificateTemplateRouter = async (server: FastifyZodProvid } }); - return { certificateTemplate }; + return certificateTemplate; } }); @@ -87,9 +70,7 @@ export const registerCertificateTemplateRouter = async (server: FastifyZodProvid .describe(CERTIFICATE_TEMPLATES.CREATE.ttl) }), response: { - 200: z.object({ - certificateTemplate: sanitizedCertificateTemplate - }) + 200: sanitizedCertificateTemplate } }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), @@ -108,12 +89,18 @@ export const registerCertificateTemplateRouter = async (server: FastifyZodProvid event: { type: EventType.CREATE_CERTIFICATE_TEMPLATE, metadata: { - certificateTemplateId: certificateTemplate.id + certificateTemplateId: certificateTemplate.id, + caId: certificateTemplate.caId, + pkiCollectionId: certificateTemplate.pkiCollectionId as string, + name: certificateTemplate.name, + commonName: certificateTemplate.commonName, + subjectAlternativeName: certificateTemplate.subjectAlternativeName, + ttl: certificateTemplate.ttl } } }); - return { certificateTemplate }; + return certificateTemplate; } }); @@ -142,9 +129,7 @@ export const registerCertificateTemplateRouter = async (server: FastifyZodProvid certificateTemplateId: z.string().describe(CERTIFICATE_TEMPLATES.UPDATE.certificateTemplateId) }), response: { - 200: z.object({ - certificateTemplate: sanitizedCertificateTemplate - }) + 200: sanitizedCertificateTemplate } }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), @@ -164,12 +149,18 @@ export const registerCertificateTemplateRouter = async (server: FastifyZodProvid event: { type: EventType.UPDATE_CERTIFICATE_TEMPLATE, metadata: { - certificateTemplateId: certificateTemplate.id + certificateTemplateId: certificateTemplate.id, + caId: certificateTemplate.caId, + pkiCollectionId: certificateTemplate.pkiCollectionId as string, + name: certificateTemplate.name, + commonName: certificateTemplate.commonName, + subjectAlternativeName: certificateTemplate.subjectAlternativeName, + ttl: certificateTemplate.ttl } } }); - return { certificateTemplate }; + return certificateTemplate; } }); @@ -184,9 +175,7 @@ export const registerCertificateTemplateRouter = async (server: FastifyZodProvid certificateTemplateId: z.string().describe(CERTIFICATE_TEMPLATES.DELETE.certificateTemplateId) }), response: { - 200: z.object({ - certificateTemplate: sanitizedCertificateTemplate - }) + 200: sanitizedCertificateTemplate } }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), @@ -210,7 +199,7 @@ export const registerCertificateTemplateRouter = async (server: FastifyZodProvid } }); - return { certificateTemplate }; + return certificateTemplate; } }); }; diff --git a/backend/src/server/routes/v2/project-router.ts b/backend/src/server/routes/v2/project-router.ts index 307ce3b3b..61e175cfd 100644 --- a/backend/src/server/routes/v2/project-router.ts +++ b/backend/src/server/routes/v2/project-router.ts @@ -4,7 +4,6 @@ import { z } from "zod"; import { CertificateAuthoritiesSchema, CertificatesSchema, - CertificateTemplatesSchema, PkiAlertsSchema, PkiCollectionsSchema, ProjectKeysSchema @@ -16,6 +15,7 @@ import { getTelemetryDistinctId } from "@app/server/lib/telemetry"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { AuthMode } from "@app/services/auth/auth-type"; import { CaStatus } from "@app/services/certificate-authority/certificate-authority-types"; +import { sanitizedCertificateTemplate } from "@app/services/certificate-template/certificate-template-schema"; import { ProjectFilterType } from "@app/services/project/project-types"; import { PostHogEventTypes } from "@app/services/telemetry/telemetry-types"; @@ -472,17 +472,7 @@ export const registerProjectRouter = async (server: FastifyZodProvider) => { }), response: { 200: z.object({ - certificateTemplates: z.array( - CertificateTemplatesSchema.pick({ - id: true, - name: true - }).merge( - z.object({ - caName: z.string(), - caId: z.string() - }) - ) - ) + certificateTemplates: sanitizedCertificateTemplate.array() }) } }, diff --git a/backend/src/services/certificate-template/certificate-template-dal.ts b/backend/src/services/certificate-template/certificate-template-dal.ts index 478a1a3dd..a30c4890d 100644 --- a/backend/src/services/certificate-template/certificate-template-dal.ts +++ b/backend/src/services/certificate-template/certificate-template-dal.ts @@ -1,5 +1,6 @@ import { TDbClient } from "@app/db"; import { TableName } from "@app/db/schemas"; +import { DatabaseError } from "@app/lib/errors"; import { ormify, selectAllTableCols } from "@app/lib/knex"; export type TCertificateTemplateDALFactory = ReturnType; @@ -8,37 +9,48 @@ export const certificateTemplateDALFactory = (db: TDbClient) => { const certificateTemplateOrm = ormify(db, TableName.CertificateTemplate); const getCertTemplatesByProjectId = async (projectId: string) => { - const certTemplates = await db - .replicaNode()(TableName.CertificateTemplate) - .join( - TableName.CertificateAuthority, - `${TableName.CertificateAuthority}.id`, - `${TableName.CertificateTemplate}.caId` - ) - .where(`${TableName.CertificateAuthority}.projectId`, "=", projectId) - .select(selectAllTableCols(TableName.CertificateTemplate)) - .select(db.ref("friendlyName").as("caName").withSchema(TableName.CertificateAuthority)); + try { + const certTemplates = await db + .replicaNode()(TableName.CertificateTemplate) + .join( + TableName.CertificateAuthority, + `${TableName.CertificateAuthority}.id`, + `${TableName.CertificateTemplate}.caId` + ) + .where(`${TableName.CertificateAuthority}.projectId`, "=", projectId) + .select(selectAllTableCols(TableName.CertificateTemplate)) + .select( + db.ref("friendlyName").as("caName").withSchema(TableName.CertificateAuthority), + db.ref("projectId").withSchema(TableName.CertificateAuthority) + ); - return certTemplates; + return certTemplates; + } catch (error) { + throw new DatabaseError({ error, name: "Get certificate templates by project ID" }); + } }; const getById = async (id: string) => { - const certTemplate = await db - .replicaNode()(TableName.CertificateTemplate) - .join( - TableName.CertificateAuthority, - `${TableName.CertificateAuthority}.id`, - `${TableName.CertificateTemplate}.caId` - ) - .where(`${TableName.CertificateTemplate}.id`, "=", id) - .select(selectAllTableCols(TableName.CertificateTemplate)) - .select( - db.ref("projectId").withSchema(TableName.CertificateAuthority), - db.ref("friendlyName").as("caName").withSchema(TableName.CertificateAuthority) - ) - .first(); + try { + const certTemplate = await db + .replicaNode()(TableName.CertificateTemplate) + .join( + TableName.CertificateAuthority, + `${TableName.CertificateAuthority}.id`, + `${TableName.CertificateTemplate}.caId` + ) + .where(`${TableName.CertificateTemplate}.id`, "=", id) + .select(selectAllTableCols(TableName.CertificateTemplate)) + .select( + db.ref("projectId").withSchema(TableName.CertificateAuthority), + db.ref("friendlyName").as("caName").withSchema(TableName.CertificateAuthority) + ) + .first(); - return certTemplate; + return certTemplate; + } catch (error) { + throw new DatabaseError({ error, name: "Get certificate template by ID" }); + } }; return { ...certificateTemplateOrm, getCertTemplatesByProjectId, getById }; diff --git a/backend/src/services/certificate-template/certificate-template-schema.ts b/backend/src/services/certificate-template/certificate-template-schema.ts new file mode 100644 index 000000000..2ce787050 --- /dev/null +++ b/backend/src/services/certificate-template/certificate-template-schema.ts @@ -0,0 +1,18 @@ +import z from "zod"; + +import { CertificateTemplatesSchema } from "@app/db/schemas"; + +export const sanitizedCertificateTemplate = CertificateTemplatesSchema.pick({ + id: true, + caId: true, + name: true, + commonName: true, + subjectAlternativeName: true, + pkiCollectionId: true, + ttl: true +}).merge( + z.object({ + projectId: z.string(), + caName: z.string() + }) +); diff --git a/backend/src/services/certificate-template/certificate-template-service.ts b/backend/src/services/certificate-template/certificate-template-service.ts index 459c988d8..c49791175 100644 --- a/backend/src/services/certificate-template/certificate-template-service.ts +++ b/backend/src/services/certificate-template/certificate-template-service.ts @@ -57,7 +57,7 @@ export const certificateTemplateServiceFactory = ({ ProjectPermissionSub.CertificateTemplates ); - const certificateTemplate = await certificateTemplateDAL.create({ + const { id } = await certificateTemplateDAL.create({ caId, pkiCollectionId, name, @@ -66,7 +66,14 @@ export const certificateTemplateServiceFactory = ({ ttl }); - return { ...certificateTemplate, projectId: ca.projectId }; + const certificateTemplate = await certificateTemplateDAL.getById(id); + if (!certificateTemplate) { + throw new NotFoundError({ + message: "Certificate template not found" + }); + } + + return certificateTemplate; }; const updateCertTemplate = async ({ @@ -111,7 +118,7 @@ export const certificateTemplateServiceFactory = ({ } } - const updatedCertTemplate = await certificateTemplateDAL.updateById(certTemplate.id, { + await certificateTemplateDAL.updateById(certTemplate.id, { caId, pkiCollectionId, commonName, @@ -120,7 +127,14 @@ export const certificateTemplateServiceFactory = ({ ttl }); - return { ...updatedCertTemplate, projectId: certTemplate.projectId }; + const updatedTemplate = await certificateTemplateDAL.getById(id); + if (!updatedTemplate) { + throw new NotFoundError({ + message: "Certificate template not found" + }); + } + + return updatedTemplate; }; const deleteCertTemplate = async ({ id, actorId, actorAuthMethod, actor, actorOrgId }: TDeleteCertTemplateDTO) => { @@ -144,9 +158,9 @@ export const certificateTemplateServiceFactory = ({ ProjectPermissionSub.CertificateTemplates ); - const deletedCertTemplate = await certificateTemplateDAL.deleteById(certTemplate.id); + await certificateTemplateDAL.deleteById(certTemplate.id); - return { ...deletedCertTemplate, projectId: certTemplate.projectId }; + return certTemplate; }; const getCertTemplate = async ({ id, actorId, actorAuthMethod, actor, actorOrgId }: TGetCertTemplateDTO) => { diff --git a/frontend/src/hooks/api/auditLogs/types.tsx b/frontend/src/hooks/api/auditLogs/types.tsx index ddc0e887e..d7825637d 100644 --- a/frontend/src/hooks/api/auditLogs/types.tsx +++ b/frontend/src/hooks/api/auditLogs/types.tsx @@ -683,6 +683,12 @@ interface CreateCertificateTemplate { type: EventType.CREATE_CERTIFICATE_TEMPLATE; metadata: { certificateTemplateId: string; + caId: string; + pkiCollectionId?: string; + name: string; + commonName: string; + subjectAlternativeName: string; + ttl: string; }; } @@ -697,6 +703,12 @@ interface UpdateCertificateTemplate { type: EventType.UPDATE_CERTIFICATE_TEMPLATE; metadata: { certificateTemplateId: string; + caId: string; + pkiCollectionId?: string; + name: string; + commonName: string; + subjectAlternativeName: string; + ttl: string; }; } diff --git a/frontend/src/hooks/api/certificateTemplates/mutations.tsx b/frontend/src/hooks/api/certificateTemplates/mutations.tsx index 89ea225f2..269d885a6 100644 --- a/frontend/src/hooks/api/certificateTemplates/mutations.tsx +++ b/frontend/src/hooks/api/certificateTemplates/mutations.tsx @@ -15,9 +15,7 @@ export const useCreateCertTemplate = () => { const queryClient = useQueryClient(); return useMutation({ mutationFn: async (data) => { - const { - data: { certificateTemplate } - } = await apiRequest.post<{ certificateTemplate: TCertificateTemplate }>( + const { data: certificateTemplate } = await apiRequest.post( "/api/v1/pki/certificate-templates", data ); @@ -33,9 +31,7 @@ export const useUpdateCertTemplate = () => { const queryClient = useQueryClient(); return useMutation({ mutationFn: async (data) => { - const { - data: { certificateTemplate } - } = await apiRequest.patch<{ certificateTemplate: TCertificateTemplate }>( + const { data: certificateTemplate } = await apiRequest.patch( `/api/v1/pki/certificate-templates/${data.id}`, data ); diff --git a/frontend/src/hooks/api/certificateTemplates/queries.tsx b/frontend/src/hooks/api/certificateTemplates/queries.tsx index 56e3e5d54..ca5b088e8 100644 --- a/frontend/src/hooks/api/certificateTemplates/queries.tsx +++ b/frontend/src/hooks/api/certificateTemplates/queries.tsx @@ -12,9 +12,7 @@ export const useGetCertTemplate = (id: string) => { return useQuery({ queryKey: certTemplateKeys.getCertTemplateById(id), queryFn: async () => { - const { - data: { certificateTemplate } - } = await apiRequest.get<{ certificateTemplate: TCertificateTemplate }>( + const { data: certificateTemplate } = await apiRequest.get( `/api/v1/pki/certificate-templates/${id}` ); return certificateTemplate; diff --git a/frontend/src/hooks/api/certificateTemplates/types.ts b/frontend/src/hooks/api/certificateTemplates/types.ts index b7cd57a3d..237a86a5e 100644 --- a/frontend/src/hooks/api/certificateTemplates/types.ts +++ b/frontend/src/hooks/api/certificateTemplates/types.ts @@ -1,13 +1,8 @@ -export type TCertificateTemplateListEntry = { - id: string; - name: string; - caName: string; - caId: string; -}; - export type TCertificateTemplate = { id: string; caId: string; + caName: string; + projectId: string; pkiCollectionId?: string; name: string; commonName: string; diff --git a/frontend/src/hooks/api/workspace/queries.tsx b/frontend/src/hooks/api/workspace/queries.tsx index 711641be8..277499e9b 100644 --- a/frontend/src/hooks/api/workspace/queries.tsx +++ b/frontend/src/hooks/api/workspace/queries.tsx @@ -5,7 +5,7 @@ import { apiRequest } from "@app/config/request"; import { CaStatus } from "../ca/enums"; import { TCertificateAuthority } from "../ca/types"; import { TCertificate } from "../certificates/types"; -import { TCertificateTemplateListEntry } from "../certificateTemplates/types"; +import { TCertificateTemplate } from "../certificateTemplates/types"; import { TGroupMembership } from "../groups/types"; import { identitiesKeys } from "../identities/queries"; import { IdentityMembership } from "../identities/types"; @@ -649,7 +649,7 @@ export const useListWorkspaceCertificateTemplates = ({ workspaceId }: { workspac queryFn: async () => { const { data: { certificateTemplates } - } = await apiRequest.get<{ certificateTemplates: TCertificateTemplateListEntry[] }>( + } = await apiRequest.get<{ certificateTemplates: TCertificateTemplate[] }>( `/api/v2/workspace/${workspaceId}/certificate-templates` ); diff --git a/frontend/src/views/Project/AuditLogsPage/components/LogsTableRow.tsx b/frontend/src/views/Project/AuditLogsPage/components/LogsTableRow.tsx index ed25abe79..ea0e687bb 100644 --- a/frontend/src/views/Project/AuditLogsPage/components/LogsTableRow.tsx +++ b/frontend/src/views/Project/AuditLogsPage/components/LogsTableRow.tsx @@ -408,6 +408,19 @@ export const LogsTableRow = ({ auditLog }: Props) => { ); case EventType.CREATE_CERTIFICATE_TEMPLATE: case EventType.UPDATE_CERTIFICATE_TEMPLATE: + return ( + +

{`Certificate Template ID: ${event.metadata.certificateTemplateId}`}

+

{`Certificate Authority ID: ${event.metadata.caId}`}

+

{`Name: ${event.metadata.name}`}

+

{`Common Name: ${event.metadata.commonName}`}

+

{`Subject Alternative Name: ${event.metadata.subjectAlternativeName}`}

+

{`TTL: ${event.metadata.ttl}`}

+ {event.metadata.pkiCollectionId && ( +

{`Collection ID: ${event.metadata.pkiCollectionId}`}

+ )} + + ); case EventType.GET_CERTIFICATE_TEMPLATE: case EventType.DELETE_CERTIFICATE_TEMPLATE: return (