From 30d6af7760acf5468ae1147b029ce8c22b5fd363 Mon Sep 17 00:00:00 2001 From: Tuan Dang Date: Sat, 17 Aug 2024 21:23:25 -0700 Subject: [PATCH] Make PR review adjustments --- .../20240802181855_ca-cert-version.ts | 2 +- ...ing.ts => 20240818024923_cert-alerting.ts} | 5 +- backend/src/queue/queue-service.ts | 6 +++ backend/src/server/routes/index.ts | 8 ++- backend/src/server/routes/v1/index.ts | 4 +- .../src/server/routes/v1/pki-alert-router.ts | 8 ++- .../certificate-authority-fns.ts | 12 ++--- .../expiring-pki-item-alert-queue.ts | 48 +++++++++++++++++ .../services/pki-alert/pki-alert-service.ts | 7 ++- .../pki-collection/pki-collection-item-dal.ts | 2 +- .../pki-collection/pki-collection-service.ts | 13 +++-- .../resource-cleanup-queue.ts | 4 -- .../components/CertificateModal.tsx | 52 ++++++++++--------- .../components/PkiCollectionItemsTable.tsx | 4 +- 14 files changed, 124 insertions(+), 51 deletions(-) rename backend/src/db/migrations/{20240806173521_cert-alerting.ts => 20240818024923_cert-alerting.ts} (97%) create mode 100644 backend/src/services/pki-alert/expiring-pki-item-alert-queue.ts diff --git a/backend/src/db/migrations/20240802181855_ca-cert-version.ts b/backend/src/db/migrations/20240802181855_ca-cert-version.ts index b9c572ecd..24eca185d 100644 --- a/backend/src/db/migrations/20240802181855_ca-cert-version.ts +++ b/backend/src/db/migrations/20240802181855_ca-cert-version.ts @@ -25,7 +25,7 @@ export async function up(knex: Knex): Promise { if (!hasVersionColumn) { await knex.schema.alterTable(TableName.CertificateAuthorityCert, (t) => { t.integer("version").nullable(); - // t.dropUnique(["caId"]); + t.dropUnique(["caId"]); }); await knex(TableName.CertificateAuthorityCert).update({ version: 1 }).whereNull("version"); diff --git a/backend/src/db/migrations/20240806173521_cert-alerting.ts b/backend/src/db/migrations/20240818024923_cert-alerting.ts similarity index 97% rename from backend/src/db/migrations/20240806173521_cert-alerting.ts rename to backend/src/db/migrations/20240818024923_cert-alerting.ts index 5ea98cda9..f60ce8c02 100644 --- a/backend/src/db/migrations/20240806173521_cert-alerting.ts +++ b/backend/src/db/migrations/20240818024923_cert-alerting.ts @@ -15,6 +15,8 @@ export async function up(knex: Knex): Promise { }); } + await createOnUpdateTrigger(knex, TableName.PkiCollection); + if (!(await knex.schema.hasTable(TableName.PkiCollectionItem))) { await knex.schema.createTable(TableName.PkiCollectionItem, (t) => { t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); @@ -28,6 +30,8 @@ export async function up(knex: Knex): Promise { }); } + await createOnUpdateTrigger(knex, TableName.PkiCollectionItem); + if (!(await knex.schema.hasTable(TableName.PkiAlert))) { await knex.schema.createTable(TableName.PkiAlert, (t) => { t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); @@ -43,7 +47,6 @@ export async function up(knex: Knex): Promise { }); } - await createOnUpdateTrigger(knex, TableName.PkiCollection); await createOnUpdateTrigger(knex, TableName.PkiAlert); } diff --git a/backend/src/queue/queue-service.ts b/backend/src/queue/queue-service.ts index ec222f0fa..48330cf04 100644 --- a/backend/src/queue/queue-service.ts +++ b/backend/src/queue/queue-service.ts @@ -16,6 +16,7 @@ export enum QueueName { // TODO(akhilmhdh): This will get removed later. For now this is kept to stop the repeatable queue AuditLogPrune = "audit-log-prune", DailyResourceCleanUp = "daily-resource-cleanup", + DailyExpiringPkiItemAlert = "daily-expiring-pki-item-alert", TelemetryInstanceStats = "telemtry-self-hosted-stats", IntegrationSync = "sync-integrations", SecretWebhook = "secret-webhook", @@ -36,6 +37,7 @@ export enum QueueJobs { // TODO(akhilmhdh): This will get removed later. For now this is kept to stop the repeatable queue AuditLogPrune = "audit-log-prune-job", DailyResourceCleanUp = "daily-resource-cleanup-job", + DailyExpiringPkiItemAlert = "daily-expiring-pki-item-alert", SecWebhook = "secret-webhook-trigger", TelemetryInstanceStats = "telemetry-self-hosted-stats", IntegrationSync = "secret-integration-pull", @@ -71,6 +73,10 @@ export type TQueueJobTypes = { name: QueueJobs.DailyResourceCleanUp; payload: undefined; }; + [QueueName.DailyExpiringPkiItemAlert]: { + name: QueueJobs.DailyExpiringPkiItemAlert; + payload: undefined; + }; [QueueName.AuditLogPrune]: { name: QueueJobs.AuditLogPrune; payload: undefined; diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index a630e0441..1aa1e18fc 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -131,6 +131,7 @@ import { orgRoleServiceFactory } from "@app/services/org/org-role-service"; import { orgServiceFactory } from "@app/services/org/org-service"; import { orgAdminServiceFactory } from "@app/services/org-admin/org-admin-service"; import { orgMembershipDALFactory } from "@app/services/org-membership/org-membership-dal"; +import { dailyExpiringPkiItemAlertQueueServiceFactory } from "@app/services/pki-alert/expiring-pki-item-alert-queue"; import { pkiAlertDALFactory } from "@app/services/pki-alert/pki-alert-dal"; import { pkiAlertServiceFactory } from "@app/services/pki-alert/pki-alert-service"; import { pkiCollectionDALFactory } from "@app/services/pki-collection/pki-collection-dal"; @@ -1063,7 +1064,6 @@ export const registerRoutes = async ( const dailyResourceCleanUp = dailyResourceCleanUpQueueServiceFactory({ auditLogDAL, queueService, - pkiAlertService, secretVersionDAL, secretFolderVersionDAL: folderVersionDAL, snapshotDAL, @@ -1073,6 +1073,11 @@ export const registerRoutes = async ( identityUniversalAuthClientSecretDAL: identityUaClientSecretDAL }); + const dailyExpiringPkiItemAlert = dailyExpiringPkiItemAlertQueueServiceFactory({ + queueService, + pkiAlertService + }); + const oidcService = oidcConfigServiceFactory({ orgDAL, orgMembershipDAL, @@ -1097,6 +1102,7 @@ export const registerRoutes = async ( await telemetryQueue.startTelemetryCheck(); await dailyResourceCleanUp.startCleanUp(); + await dailyExpiringPkiItemAlert.startSendingAlerts(); await kmsService.startService(); // inject all services diff --git a/backend/src/server/routes/v1/index.ts b/backend/src/server/routes/v1/index.ts index 1a5a56bd0..01bec65ad 100644 --- a/backend/src/server/routes/v1/index.ts +++ b/backend/src/server/routes/v1/index.ts @@ -76,8 +76,8 @@ export const registerV1Routes = async (server: FastifyZodProvider) => { async (pkiRouter) => { await pkiRouter.register(registerCaRouter, { prefix: "/ca" }); await pkiRouter.register(registerCertRouter, { prefix: "/certificates" }); - await server.register(registerPkiAlertRouter, { prefix: "/alerts" }); - await server.register(registerPkiCollectionRouter, { prefix: "/collections" }); + await pkiRouter.register(registerPkiAlertRouter, { prefix: "/alerts" }); + await pkiRouter.register(registerPkiCollectionRouter, { prefix: "/collections" }); }, { prefix: "/pki" } ); diff --git a/backend/src/server/routes/v1/pki-alert-router.ts b/backend/src/server/routes/v1/pki-alert-router.ts index afaa99bdf..f64ec9e47 100644 --- a/backend/src/server/routes/v1/pki-alert-router.ts +++ b/backend/src/server/routes/v1/pki-alert-router.ts @@ -22,7 +22,11 @@ export const registerPkiAlertRouter = async (server: FastifyZodProvider) => { pkiCollectionId: z.string().trim().describe(ALERTS.CREATE.pkiCollectionId), name: z.string().trim().describe(ALERTS.CREATE.name), alertBeforeDays: z.number().describe(ALERTS.CREATE.alertBeforeDays), - emails: z.array(z.string().trim().email({ message: "Invalid email address" })).describe(ALERTS.CREATE.emails) + emails: z + .array(z.string().trim().email({ message: "Invalid email address" })) + .min(1, { message: "You must specify at least 1 email" }) + .max(5, { message: "You can specify a maximum of 5 emails" }) + .describe(ALERTS.CREATE.emails) }), response: { 200: PkiAlertsSchema @@ -114,6 +118,8 @@ export const registerPkiAlertRouter = async (server: FastifyZodProvider) => { pkiCollectionId: z.string().trim().optional().describe(ALERTS.UPDATE.pkiCollectionId), emails: z .array(z.string().trim().email({ message: "Invalid email address" })) + .min(1, { message: "You must specify at least 1 email" }) + .max(5, { message: "You can specify a maximum of 5 emails" }) .optional() .describe(ALERTS.UPDATE.emails) }), diff --git a/backend/src/services/certificate-authority/certificate-authority-fns.ts b/backend/src/services/certificate-authority/certificate-authority-fns.ts index 4e3df1173..b1fd87a26 100644 --- a/backend/src/services/certificate-authority/certificate-authority-fns.ts +++ b/backend/src/services/certificate-authority/certificate-authority-fns.ts @@ -1,7 +1,7 @@ import * as x509 from "@peculiar/x509"; import crypto from "crypto"; -import { BadRequestError } from "@app/lib/errors"; +import { NotFoundError } from "@app/lib/errors"; import { getProjectKmsCertificateKeyId } from "@app/services/project/project-fns"; import { CertKeyAlgorithm, CertStatus } from "../certificate/certificate-types"; @@ -106,10 +106,10 @@ export const getCaCredentials = async ({ kmsService }: TGetCaCredentialsDTO) => { const ca = await certificateAuthorityDAL.findById(caId); - if (!ca) throw new BadRequestError({ message: "CA not found" }); + if (!ca) throw new NotFoundError({ message: "CA not found" }); const caSecret = await certificateAuthoritySecretDAL.findOne({ caId }); - if (!caSecret) throw new BadRequestError({ message: "CA secret not found" }); + if (!caSecret) throw new NotFoundError({ message: "CA secret not found" }); const keyId = await getProjectKmsCertificateKeyId({ projectId: ca.projectId, @@ -158,7 +158,7 @@ export const getCaCertChains = async ({ kmsService }: TGetCaCertChainsDTO) => { const ca = await certificateAuthorityDAL.findById(caId); - if (!ca) throw new BadRequestError({ message: "CA not found" }); + if (!ca) throw new NotFoundError({ message: "CA not found" }); const keyId = await getProjectKmsCertificateKeyId({ projectId: ca.projectId, @@ -205,7 +205,7 @@ export const getCaCertChain = async ({ kmsService }: TGetCaCertChainDTO) => { const caCert = await certificateAuthorityCertDAL.findById(caCertId); - if (!caCert) throw new BadRequestError({ message: "CA certificate not found" }); + if (!caCert) throw new NotFoundError({ message: "CA certificate not found" }); const ca = await certificateAuthorityDAL.findById(caCert.caId); const keyId = await getProjectKmsCertificateKeyId({ @@ -249,7 +249,7 @@ export const rebuildCaCrl = async ({ kmsService }: TRebuildCaCrlDTO) => { const ca = await certificateAuthorityDAL.findById(caId); - if (!ca) throw new BadRequestError({ message: "CA not found" }); + if (!ca) throw new NotFoundError({ message: "CA not found" }); const caSecret = await certificateAuthoritySecretDAL.findOne({ caId: ca.id }); diff --git a/backend/src/services/pki-alert/expiring-pki-item-alert-queue.ts b/backend/src/services/pki-alert/expiring-pki-item-alert-queue.ts new file mode 100644 index 000000000..a592e9271 --- /dev/null +++ b/backend/src/services/pki-alert/expiring-pki-item-alert-queue.ts @@ -0,0 +1,48 @@ +import { logger } from "@app/lib/logger"; +import { QueueJobs, QueueName, TQueueServiceFactory } from "@app/queue"; +import { TPkiAlertServiceFactory } from "@app/services/pki-alert/pki-alert-service"; + +type TDailyExpiringPkiItemAlertQueueServiceFactoryDep = { + queueService: TQueueServiceFactory; + pkiAlertService: Pick; +}; + +export type TDailyExpiringPkiItemAlertQueueServiceFactory = ReturnType< + typeof dailyExpiringPkiItemAlertQueueServiceFactory +>; + +export const dailyExpiringPkiItemAlertQueueServiceFactory = ({ + queueService, + pkiAlertService +}: TDailyExpiringPkiItemAlertQueueServiceFactoryDep) => { + queueService.start(QueueName.DailyExpiringPkiItemAlert, async () => { + logger.info(`${QueueName.DailyExpiringPkiItemAlert}: queue task started`); + await pkiAlertService.sendPkiItemExpiryNotices(); + logger.info(`${QueueName.DailyExpiringPkiItemAlert}: queue task completed`); + }); + + // we do a repeat cron job in utc timezone at 12 Midnight each day + const startSendingAlerts = async () => { + // clear previous job + await queueService.stopRepeatableJob( + QueueName.DailyExpiringPkiItemAlert, + QueueJobs.DailyExpiringPkiItemAlert, + { pattern: "0 0 * * *", utc: true }, + QueueName.DailyExpiringPkiItemAlert // just a job id + ); + + await queueService.queue(QueueName.DailyExpiringPkiItemAlert, QueueJobs.DailyExpiringPkiItemAlert, undefined, { + delay: 5000, + jobId: QueueName.DailyExpiringPkiItemAlert, + repeat: { pattern: "0 0 * * *", utc: true } + }); + }; + + queueService.listen(QueueName.DailyExpiringPkiItemAlert, "failed", (_, err) => { + logger.error(err, `${QueueName.DailyExpiringPkiItemAlert}: Expiring PKI item alert failed`); + }); + + return { + startSendingAlerts + }; +}; diff --git a/backend/src/services/pki-alert/pki-alert-service.ts b/backend/src/services/pki-alert/pki-alert-service.ts index 44f029d42..26ab380fb 100644 --- a/backend/src/services/pki-alert/pki-alert-service.ts +++ b/backend/src/services/pki-alert/pki-alert-service.ts @@ -12,8 +12,11 @@ import { TPkiAlertDALFactory } from "./pki-alert-dal"; import { TCreateAlertDTO, TDeleteAlertDTO, TGetAlertByIdDTO, TUpdateAlertDTO } from "./pki-alert-types"; type TPkiAlertServiceFactoryDep = { - pkiAlertDAL: TPkiAlertDALFactory; - pkiCollectionDAL: TPkiCollectionDALFactory; + pkiAlertDAL: Pick< + TPkiAlertDALFactory, + "create" | "findById" | "updateById" | "deleteById" | "getExpiringPkiCollectionItemsForAlerting" + >; + pkiCollectionDAL: Pick; permissionService: Pick; smtpService: Pick; }; diff --git a/backend/src/services/pki-collection/pki-collection-item-dal.ts b/backend/src/services/pki-collection/pki-collection-item-dal.ts index 403fd928b..de896e15c 100644 --- a/backend/src/services/pki-collection/pki-collection-item-dal.ts +++ b/backend/src/services/pki-collection/pki-collection-item-dal.ts @@ -81,7 +81,7 @@ export const pkiCollectionItemDALFactory = (db: TDbClient) => { return parseInt((count as unknown as CountResult).count || "0", 10); } catch (error) { - throw new DatabaseError({ error, name: "Count all project certificates" }); + throw new DatabaseError({ error, name: "Count all PKI collection items" }); } }; diff --git a/backend/src/services/pki-collection/pki-collection-service.ts b/backend/src/services/pki-collection/pki-collection-service.ts index f7c7ba399..f2a3fa0c4 100644 --- a/backend/src/services/pki-collection/pki-collection-service.ts +++ b/backend/src/services/pki-collection/pki-collection-service.ts @@ -22,10 +22,13 @@ import { } from "./pki-collection-types"; type TPkiCollectionServiceFactoryDep = { - pkiCollectionDAL: TPkiCollectionDALFactory; // TODO: Pick - pkiCollectionItemDAL: TPkiCollectionItemDALFactory; - certificateAuthorityDAL: TCertificateAuthorityDALFactory; - certificateDAL: TCertificateDALFactory; + pkiCollectionDAL: Pick; + pkiCollectionItemDAL: Pick< + TPkiCollectionItemDALFactory, + "findOne" | "create" | "deleteById" | "findPkiCollectionItems" | "countItemsInPkiCollection" + >; + certificateAuthorityDAL: Pick; + certificateDAL: Pick; permissionService: Pick; }; @@ -139,7 +142,7 @@ export const pkiCollectionServiceFactory = ({ ); ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionActions.Create, + ProjectPermissionActions.Delete, ProjectPermissionSub.PkiCollections ); pkiCollection = await pkiCollectionDAL.deleteById(collectionId); diff --git a/backend/src/services/resource-cleanup/resource-cleanup-queue.ts b/backend/src/services/resource-cleanup/resource-cleanup-queue.ts index fffadcb06..dab70806f 100644 --- a/backend/src/services/resource-cleanup/resource-cleanup-queue.ts +++ b/backend/src/services/resource-cleanup/resource-cleanup-queue.ts @@ -2,7 +2,6 @@ import { TAuditLogDALFactory } from "@app/ee/services/audit-log/audit-log-dal"; import { TSnapshotDALFactory } from "@app/ee/services/secret-snapshot/snapshot-dal"; import { logger } from "@app/lib/logger"; import { QueueJobs, QueueName, TQueueServiceFactory } from "@app/queue"; -import { TPkiAlertServiceFactory } from "@app/services/pki-alert/pki-alert-service"; import { TIdentityAccessTokenDALFactory } from "../identity-access-token/identity-access-token-dal"; import { TIdentityUaClientSecretDALFactory } from "../identity-ua/identity-ua-client-secret-dal"; @@ -21,7 +20,6 @@ type TDailyResourceCleanUpQueueServiceFactoryDep = { snapshotDAL: Pick; secretSharingDAL: Pick; queueService: TQueueServiceFactory; - pkiAlertService: Pick; }; export type TDailyResourceCleanUpQueueServiceFactory = ReturnType; @@ -29,7 +27,6 @@ export type TDailyResourceCleanUpQueueServiceFactory = ReturnType { )} /> - ( - - - - )} - /> + + + )} + /> + )} - + )}