From 90b93fbd155531a3a6f91fd6f5346651cb9b63d0 Mon Sep 17 00:00:00 2001 From: Scott Wilson Date: Wed, 13 Aug 2025 16:03:48 -0700 Subject: [PATCH] improvements: address feedback --- .../v1/access-approval-request-router.ts | 18 ++++++- .../access-approval-request-service.ts | 12 +++-- .../trigger-notification.ts | 10 +++- .../src/lib/workflow-integrations/types.ts | 16 ++++++- .../microsoft-teams/microsoft-teams-fns.ts | 48 +++++++++++++++++++ backend/src/services/slack/slack-fns.ts | 38 +++++++++++++++ .../components/EditAccessRequestModal.tsx | 18 ++++++- .../components/ReviewAccessModal.tsx | 5 +- 8 files changed, 152 insertions(+), 13 deletions(-) diff --git a/backend/src/ee/routes/v1/access-approval-request-router.ts b/backend/src/ee/routes/v1/access-approval-request-router.ts index 217ae1f13..83378c16e 100644 --- a/backend/src/ee/routes/v1/access-approval-request-router.ts +++ b/backend/src/ee/routes/v1/access-approval-request-router.ts @@ -27,7 +27,23 @@ export const registerAccessApprovalRequestRouter = async (server: FastifyZodProv body: z.object({ permissions: z.any().array(), isTemporary: z.boolean(), - temporaryRange: z.string().optional(), + temporaryRange: z + .string() + .optional() + .transform((val, ctx) => { + if (!val || val === "permanent") return undefined; + + const parsedMs = ms(val); + + if (typeof parsedMs !== "number" || parsedMs <= 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "Invalid time period format or value. Must be a positive duration (e.g., '1h', '30m', '2d')." + }); + return z.NEVER; + } + return val; + }), note: z.string().max(255).optional() }), querystring: z.object({ diff --git a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts b/backend/src/ee/services/access-approval-request/access-approval-request-service.ts index 850e1d3a2..0f05bd5af 100644 --- a/backend/src/ee/services/access-approval-request/access-approval-request-service.ts +++ b/backend/src/ee/services/access-approval-request/access-approval-request-service.ts @@ -346,7 +346,9 @@ export const accessApprovalRequestServiceFactory = ({ const project = await projectDAL.findById(accessApprovalRequest.projectId); if (!project) { - throw new NotFoundError({ message: "The project associated with this access request was not found." }); + throw new NotFoundError({ + message: `The project associated with this access request was not found. [projectId=${accessApprovalRequest.projectId}]` + }); } if (accessApprovalRequest.status !== ApprovalStatus.PENDING) { @@ -355,7 +357,7 @@ export const accessApprovalRequestServiceFactory = ({ const editedByUser = await userDAL.findById(actorId); - if (!editedByUser) throw new ForbiddenRequestError({ message: "User not found" }); + if (!editedByUser) throw new NotFoundError({ message: "Editing user not found" }); if (accessApprovalRequest.isTemporary && accessApprovalRequest.temporaryRange) { if (ms(temporaryRange) > ms(accessApprovalRequest.temporaryRange)) { @@ -394,7 +396,7 @@ export const accessApprovalRequestServiceFactory = ({ await triggerWorkflowIntegrationNotification({ input: { notification: { - type: TriggerFeature.ACCESS_REQUEST, + type: TriggerFeature.ACCESS_REQUEST_UPDATED, payload: { projectName: project.name, requesterFullName, @@ -421,7 +423,9 @@ export const accessApprovalRequestServiceFactory = ({ }); await smtpService.sendMail({ - recipients: policy.approvers.filter((approver) => approver.email).map((approver) => approver.email!), + recipients: policy.approvers + .filter((approver) => Boolean(approver.email) && approver.userId !== editedByUser.id) + .map((approver) => approver.email!), subjectLine: "Access Approval Request Updated", substitutions: { projectName: project.name, diff --git a/backend/src/lib/workflow-integrations/trigger-notification.ts b/backend/src/lib/workflow-integrations/trigger-notification.ts index 58411bdb0..355761f73 100644 --- a/backend/src/lib/workflow-integrations/trigger-notification.ts +++ b/backend/src/lib/workflow-integrations/trigger-notification.ts @@ -20,7 +20,10 @@ export const triggerWorkflowIntegrationNotification = async (dto: TTriggerWorkfl const slackConfig = await projectSlackConfigDAL.getIntegrationDetailsByProject(projectId); if (slackConfig) { - if (notification.type === TriggerFeature.ACCESS_REQUEST) { + if ( + notification.type === TriggerFeature.ACCESS_REQUEST || + notification.type === TriggerFeature.ACCESS_REQUEST_UPDATED + ) { const targetChannelIds = slackConfig.accessRequestChannels?.split(", ") || []; if (targetChannelIds.length && slackConfig.isAccessRequestNotificationEnabled) { await sendSlackNotification({ @@ -50,7 +53,10 @@ export const triggerWorkflowIntegrationNotification = async (dto: TTriggerWorkfl } if (microsoftTeamsConfig) { - if (notification.type === TriggerFeature.ACCESS_REQUEST) { + if ( + notification.type === TriggerFeature.ACCESS_REQUEST || + notification.type === TriggerFeature.ACCESS_REQUEST_UPDATED + ) { if (microsoftTeamsConfig.isAccessRequestNotificationEnabled && microsoftTeamsConfig.accessRequestChannels) { const { success, data } = validateMicrosoftTeamsChannelsSchema.safeParse( microsoftTeamsConfig.accessRequestChannels diff --git a/backend/src/lib/workflow-integrations/types.ts b/backend/src/lib/workflow-integrations/types.ts index 4cb783cd2..f8f55eadd 100644 --- a/backend/src/lib/workflow-integrations/types.ts +++ b/backend/src/lib/workflow-integrations/types.ts @@ -6,7 +6,8 @@ import { TProjectSlackConfigDALFactory } from "@app/services/slack/project-slack export enum TriggerFeature { SECRET_APPROVAL = "secret-approval", - ACCESS_REQUEST = "access-request" + ACCESS_REQUEST = "access-request", + ACCESS_REQUEST_UPDATED = "access-request-updated" } export type TNotification = @@ -33,6 +34,19 @@ export type TNotification = permissions: string[]; approvalUrl: string; note?: string; + }; + } + | { + type: TriggerFeature.ACCESS_REQUEST_UPDATED; + payload: { + requesterFullName: string; + requesterEmail: string; + isTemporary: boolean; + secretPath: string; + environment: string; + projectName: string; + permissions: string[]; + approvalUrl: string; editNote?: string; editorFullName?: string; editorEmail?: string; diff --git a/backend/src/services/microsoft-teams/microsoft-teams-fns.ts b/backend/src/services/microsoft-teams/microsoft-teams-fns.ts index 38fc99819..e940fda54 100644 --- a/backend/src/services/microsoft-teams/microsoft-teams-fns.ts +++ b/backend/src/services/microsoft-teams/microsoft-teams-fns.ts @@ -462,6 +462,54 @@ export const buildTeamsPayload = (notification: TNotification) => { }; } + case TriggerFeature.ACCESS_REQUEST_UPDATED: { + const { payload } = notification; + + const adaptiveCard = { + type: "AdaptiveCard", + $schema: "http://adaptivecards.io/schemas/adaptive-card.json", + version: "1.5", + body: [ + { + type: "TextBlock", + text: "Updated access approval request pending for review", + weight: "Bolder", + size: "Large" + }, + { + type: "TextBlock", + text: `${payload.editorFullName} (${payload.editorEmail}) has updated the ${ + payload.isTemporary ? "temporary" : "permanent" + } access request from ${payload.requesterFullName} (${payload.requesterEmail}) to ${payload.secretPath} in the ${payload.environment} environment of ${payload.projectName}.`, + wrap: true + }, + { + type: "TextBlock", + text: `The following permissions are requested: ${payload.permissions.join(", ")}`, + wrap: true + }, + payload.editNote + ? { + type: "TextBlock", + text: `**Editor Note**: ${payload.editNote}`, + wrap: true + } + : null + ].filter(Boolean), + actions: [ + { + type: "Action.OpenUrl", + title: "View request in Infisical", + url: payload.approvalUrl + } + ] + }; + + return { + adaptiveCard + }; + } + default: { throw new BadRequestError({ message: "Teams notification type not supported." diff --git a/backend/src/services/slack/slack-fns.ts b/backend/src/services/slack/slack-fns.ts index a111d5372..88db1a84d 100644 --- a/backend/src/services/slack/slack-fns.ts +++ b/backend/src/services/slack/slack-fns.ts @@ -115,6 +115,44 @@ User Note: ${payload.note}` payloadBlocks }; } + case TriggerFeature.ACCESS_REQUEST_UPDATED: { + const { payload } = notification; + const messageBody = `${payload.editorFullName} (${payload.editorEmail}) has updated the ${ + payload.isTemporary ? "temporary" : "permanent" + } access request from ${payload.requesterFullName} (${payload.requesterEmail}) to ${payload.secretPath} in the ${payload.environment} environment of ${payload.projectName}. + +The following permissions are requested: ${payload.permissions.join(", ")} + +View the request and approve or deny it <${payload.approvalUrl}|here>.${ + payload.editNote + ? ` +Editor Note: ${payload.editNote}` + : "" + }`; + + const payloadBlocks = [ + { + type: "header", + text: { + type: "plain_text", + text: "Updated access approval request pending for review", + emoji: true + } + }, + { + type: "section", + text: { + type: "mrkdwn", + text: messageBody + } + } + ]; + + return { + payloadMessage: messageBody, + payloadBlocks + }; + } default: { throw new BadRequestError({ message: "Slack notification type not supported." diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/EditAccessRequestModal.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/EditAccessRequestModal.tsx index 3cdf8a063..c9c825a3c 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/EditAccessRequestModal.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/EditAccessRequestModal.tsx @@ -3,6 +3,7 @@ import { faWarning } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { zodResolver } from "@hookform/resolvers/zod"; import { useQueryClient } from "@tanstack/react-query"; +import ms from "ms"; import { z } from "zod"; import { TtlFormLabel } from "@app/components/features"; @@ -27,7 +28,22 @@ type ContentProps = { }; const EditSchema = z.object({ - temporaryRange: z.string().nonempty("Required"), + temporaryRange: z + .string() + .nonempty("Required") + .transform((val, ctx) => { + const parsedMs = ms(val); + + if (typeof parsedMs !== "number" || parsedMs <= 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + "Invalid time period format or value. Must be a positive duration (e.g., '1h', '30m', '2d')." + }); + return z.NEVER; + } + return val; + }), editNote: z.string().nonempty("Required") }); diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx index 5c6cf02dc..e87054f93 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx @@ -129,9 +129,6 @@ export const ReviewAccessRequestModal = ({ if (!accessDetails.temporaryAccess.isTemporary || !accessDetails.temporaryAccess.temporaryRange) return "Permanent"; - // convert the range to human readable format - ms(ms(accessDetails.temporaryAccess.temporaryRange), { long: true }); - return `Valid for ${ms(ms(accessDetails.temporaryAccess.temporaryRange), { long: true })} after approval`; @@ -293,7 +290,7 @@ export const ReviewAccessRequestModal = ({
{getAccessLabel()} - {request.isApprover && ( + {request.isApprover && request.status === ApprovalStatus.PENDING && ( <>