feature: allow editing of access request by admin reviewers

2026-10-13 20:29:34 +00:00 · 2025-08-12 23:25:31 -07:00
parent f5a0d8be78
commit 0d3cb2d41a
15 changed files with 610 additions and 16 deletions
@@ -0,0 +1,38 @@
+import { Knex } from "knex";
+
+import { TableName } from "@app/db/schemas";
+
+export async function up(knex: Knex): Promise<void> {
+  const hasEditNoteCol = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "editNote");
+  const hasEditedByUserId = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "editedByUserId");
+
+  if (!hasEditNoteCol || !hasEditedByUserId) {
+    await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => {
+      if (!hasEditedByUserId) {
+        t.uuid("editedByUserId").nullable();
+        t.foreign("editedByUserId").references("id").inTable(TableName.Users).onDelete("SET NULL");
+      }
+
+      if (!hasEditNoteCol) {
+        t.string("editNote").nullable();
+      }
+    });
+  }
+}
+
+export async function down(knex: Knex): Promise<void> {
+  const hasEditNoteCol = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "editNote");
+  const hasEditedByUserId = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "editedByUserId");
+
+  if (hasEditNoteCol || hasEditedByUserId) {
+    await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => {
+      if (hasEditedByUserId) {
+        t.dropColumn("editedByUserId");
+      }
+
+      if (hasEditNoteCol) {
+        t.dropColumn("editNote");
+      }
+    });
+  }
+}
@@ -20,7 +20,9 @@ export const AccessApprovalRequestsSchema = z.object({
  requestedByUserId: z.string().uuid(),
  note: z.string().nullable().optional(),
  privilegeDeletedAt: z.date().nullable().optional(),
-  status: z.string().default("pending")
+  status: z.string().default("pending"),
+  editedByUserId: z.string().uuid().nullable().optional(),
+  editNote: z.string().nullable().optional()
 });

 export type TAccessApprovalRequests = z.infer<typeof AccessApprovalRequestsSchema>;
@@ -2,6 +2,7 @@ import { z } from "zod";

 import { AccessApprovalRequestsReviewersSchema, AccessApprovalRequestsSchema, UsersSchema } from "@app/db/schemas";
 import { ApprovalStatus } from "@app/ee/services/access-approval-request/access-approval-request-types";
+import { ms } from "@app/lib/ms";
 import { writeLimit } from "@app/server/config/rateLimiter";
 import { verifyAuth } from "@app/server/plugins/auth/verify-auth";
 import { AuthMode } from "@app/services/auth/auth-type";
@@ -190,4 +191,47 @@ export const registerAccessApprovalRequestRouter = async (server: FastifyZodProv
      return { review };
    }
  });
+
+  server.route({
+    url: "/:requestId",
+    method: "PATCH",
+    schema: {
+      params: z.object({
+        requestId: z.string().trim()
+      }),
+      body: z.object({
+        temporaryRange: z.string().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().max(255)
+      }),
+      response: {
+        200: z.object({
+          approval: AccessApprovalRequestsSchema
+        })
+      }
+    },
+    onRequest: verifyAuth([AuthMode.JWT]),
+    handler: async (req) => {
+      const { request } = await server.services.accessApprovalRequest.updateAccessApprovalRequest({
+        actor: req.permission.type,
+        actorId: req.permission.id,
+        actorAuthMethod: req.permission.authMethod,
+        actorOrgId: req.permission.orgId,
+        temporaryRange: req.body.temporaryRange,
+        editNote: req.body.editNote,
+        requestId: req.params.requestId
+      });
+      return { approval: request };
+    }
+  });
 };
@@ -54,7 +54,7 @@ type TSecretApprovalRequestServiceFactoryDep = {
  accessApprovalPolicyDAL: Pick<TAccessApprovalPolicyDALFactory, "findOne" | "find" | "findLastValidPolicy">;
  accessApprovalRequestReviewerDAL: Pick<
    TAccessApprovalRequestReviewerDALFactory,
-    "create" | "find" | "findOne" | "transaction"
+    "create" | "find" | "findOne" | "transaction" | "delete"
  >;
  groupDAL: Pick<TGroupDALFactory, "findAllGroupPossibleMembers">;
  projectMembershipDAL: Pick<TProjectMembershipDALFactory, "findById">;
@@ -301,6 +301,148 @@ export const accessApprovalRequestServiceFactory = ({
    return { request: approval };
  };

+  const updateAccessApprovalRequest: TAccessApprovalRequestServiceFactory["updateAccessApprovalRequest"] = async ({
+    temporaryRange,
+    actorId,
+    actor,
+    actorOrgId,
+    actorAuthMethod,
+    editNote,
+    requestId
+  }) => {
+    const cfg = getConfig();
+
+    const accessApprovalRequest = await accessApprovalRequestDAL.findById(requestId);
+    if (!accessApprovalRequest) {
+      throw new NotFoundError({ message: `Access request with ID '${requestId}' not found` });
+    }
+
+    const { policy, requestedByUser } = accessApprovalRequest;
+    if (policy.deletedAt) {
+      throw new BadRequestError({
+        message: "The policy associated with this access request has been deleted."
+      });
+    }
+
+    const { membership, hasRole } = await permissionService.getProjectPermission({
+      actor,
+      actorId,
+      projectId: accessApprovalRequest.projectId,
+      actorAuthMethod,
+      actorOrgId,
+      actionProjectType: ActionProjectType.SecretManager
+    });
+
+    if (!membership) {
+      throw new ForbiddenRequestError({ message: "You are not a member of this project" });
+    }
+
+    const isApprover = policy.approvers.find((approver) => approver.userId === actorId);
+
+    if (!hasRole(ProjectMembershipRole.Admin) && !isApprover) {
+      throw new ForbiddenRequestError({ message: "You are not authorized to modify this request" });
+    }
+
+    const project = await projectDAL.findById(accessApprovalRequest.projectId);
+
+    if (!project) {
+      throw new NotFoundError({ message: "The project associated with this access request was not found." });
+    }
+
+    if (accessApprovalRequest.status !== ApprovalStatus.PENDING) {
+      throw new BadRequestError({ message: "The request has been closed" });
+    }
+
+    const editedByUser = await userDAL.findById(actorId);
+
+    if (!editedByUser) throw new ForbiddenRequestError({ message: "User not found" });
+
+    if (accessApprovalRequest.isTemporary && accessApprovalRequest.temporaryRange) {
+      if (ms(temporaryRange) > ms(accessApprovalRequest.temporaryRange)) {
+        throw new BadRequestError({ message: "Updated access duration must be less than current access duration" });
+      }
+    }
+
+    const { envSlug, secretPath, accessTypes } = verifyRequestedPermissions({
+      permissions: accessApprovalRequest.permissions
+    });
+
+    const approval = await accessApprovalRequestDAL.transaction(async (tx) => {
+      const approvalRequest = await accessApprovalRequestDAL.updateById(
+        requestId,
+        {
+          temporaryRange,
+          isTemporary: true,
+          editNote,
+          editedByUserId: actorId
+        },
+        tx
+      );
+
+      // reset review progress
+      await accessApprovalRequestReviewerDAL.delete({
+        requestId
+      });
+
+      const requesterFullName = `${requestedByUser.firstName} ${requestedByUser.lastName}`;
+      const editorFullName = `${editedByUser.firstName} ${editedByUser.lastName}`;
+      const approvalUrl = `${cfg.SITE_URL}/projects/secret-management/${project.id}/approval`;
+
+      await triggerWorkflowIntegrationNotification({
+        input: {
+          notification: {
+            type: TriggerFeature.ACCESS_REQUEST,
+            payload: {
+              projectName: project.name,
+              requesterFullName,
+              isTemporary: true,
+              requesterEmail: requestedByUser.email as string,
+              secretPath,
+              environment: envSlug,
+              permissions: accessTypes,
+              approvalUrl,
+              editNote,
+              editorEmail: editedByUser.email as string,
+              editorFullName
+            }
+          },
+          projectId: project.id
+        },
+        dependencies: {
+          projectDAL,
+          projectSlackConfigDAL,
+          kmsService,
+          microsoftTeamsService,
+          projectMicrosoftTeamsConfigDAL
+        }
+      });
+
+      await smtpService.sendMail({
+        recipients: policy.approvers.filter((approver) => approver.email).map((approver) => approver.email!),
+        subjectLine: "Access Approval Request Updated",
+        substitutions: {
+          projectName: project.name,
+          requesterFullName,
+          requesterEmail: requestedByUser.email,
+          isTemporary: true,
+          expiresIn: msFn(ms(temporaryRange || ""), { long: true }),
+          secretPath,
+          environment: envSlug,
+          permissions: accessTypes,
+          approvalUrl,
+          editNote,
+          editorFullName,
+          editorEmail: editedByUser.email
+        },
+        template: SmtpTemplates.AccessApprovalRequestUpdated
+      });
+
+      return approvalRequest;
+    });
+
+    return { request: approval };
+  };
+
  const listApprovalRequests: TAccessApprovalRequestServiceFactory["listApprovalRequests"] = async ({
    projectSlug,
    authorUserId,
@@ -650,6 +792,7 @@ export const accessApprovalRequestServiceFactory = ({

  return {
    createAccessApprovalRequest,
+    updateAccessApprovalRequest,
    listApprovalRequests,
    reviewAccessRequest,
    getCount
@@ -30,6 +30,12 @@ export type TCreateAccessApprovalRequestDTO = {
  note?: string;
 } & Omit<TProjectPermission, "projectId">;

+export type TUpdateAccessApprovalRequestDTO = {
+  requestId: string;
+  temporaryRange: string;
+  editNote: string;
+} & Omit<TProjectPermission, "projectId">;
+
 export type TListApprovalRequestsDTO = {
  projectSlug: string;
  authorUserId?: string;
@@ -54,6 +60,23 @@ export interface TAccessApprovalRequestServiceFactory {
      privilegeDeletedAt?: Date | null | undefined;
    };
  }>;
+  updateAccessApprovalRequest: (arg: TUpdateAccessApprovalRequestDTO) => Promise<{
+    request: {
+      status: string;
+      id: string;
+      createdAt: Date;
+      updatedAt: Date;
+      policyId: string;
+      isTemporary: boolean;
+      requestedByUserId: string;
+      privilegeId?: string | null | undefined;
+      requestedBy?: string | null | undefined;
+      temporaryRange?: string | null | undefined;
+      permissions?: unknown;
+      note?: string | null | undefined;
+      privilegeDeletedAt?: Date | null | undefined;
+    };
+  }>;
  listApprovalRequests: (arg: TListApprovalRequestsDTO) => Promise<{
    requests: {
      policy: {
@@ -33,6 +33,9 @@ export type TNotification =
        permissions: string[];
        approvalUrl: string;
        note?: string;
+        editNote?: string;
+        editorFullName?: string;
+        editorEmail?: string;
      };
    };

@@ -0,0 +1,95 @@
+import { Heading, Section, Text } from "@react-email/components";
+import React from "react";
+
+import { BaseButton } from "./BaseButton";
+import { BaseEmailWrapper, BaseEmailWrapperProps } from "./BaseEmailWrapper";
+import { BaseLink } from "./BaseLink";
+
+interface AccessApprovalRequestUpdatedTemplateProps
+  extends Omit<BaseEmailWrapperProps, "title" | "preview" | "children"> {
+  projectName: string;
+  requesterFullName: string;
+  requesterEmail: string;
+  isTemporary: boolean;
+  secretPath: string;
+  environment: string;
+  expiresIn: string;
+  permissions: string[];
+  editNote: string;
+  editorFullName: string;
+  editorEmail: string;
+  approvalUrl: string;
+}
+
+export const AccessApprovalRequestUpdatedTemplate = ({
+  projectName,
+  siteUrl,
+  requesterFullName,
+  requesterEmail,
+  isTemporary,
+  secretPath,
+  environment,
+  expiresIn,
+  permissions,
+  editNote,
+  editorEmail,
+  editorFullName,
+  approvalUrl
+}: AccessApprovalRequestUpdatedTemplateProps) => {
+  return (
+    <BaseEmailWrapper
+      title="Access Approval Request Update"
+      preview="An access approval request was updated and requires your review."
+      siteUrl={siteUrl}
+    >
+      <Heading className="text-black text-[18px] leading-[28px] text-center font-normal p-0 mx-0">
+        An access approval request was updated and is pending your review for the project <strong>{projectName}</strong>
+      </Heading>
+      <Section className="px-[24px] mb-[28px] mt-[36px] pt-[12px] pb-[8px] border border-solid border-gray-200 rounded-md bg-gray-50">
+        <Text className="text-black text-[14px] leading-[24px]">
+          <strong>{editorFullName}</strong> (<BaseLink href={`mailto:${editorEmail}`}>{editorEmail}</BaseLink>) has
+          updated the access request submitted by <strong>{requesterFullName}</strong> (
+          <BaseLink href={`mailto:${requesterEmail}`}>{requesterEmail}</BaseLink>) for <strong>{secretPath}</strong> in
+          the <strong>{environment}</strong> environment.
+        </Text>
+
+        {isTemporary && (
+          <Text className="text-[14px] text-red-600 leading-[24px]">
+            <strong>This access will expire {expiresIn} after approval.</strong>
+          </Text>
+        )}
+        <Text className="text-[14px] leading-[24px] mb-[4px]">
+          <strong>The following permissions are requested:</strong>
+        </Text>
+        {permissions.map((permission) => (
+          <Text key={permission} className="text-[14px] my-[2px] leading-[24px]">
+            - {permission}
+          </Text>
+        ))}
+        <Text className="text-[14px] text-slate-700 leading-[24px]">
+          <strong className="text-black">Editor Note:</strong> "{editNote}"
+        </Text>
+      </Section>
+      <Section className="text-center">
+        <BaseButton href={approvalUrl}>Review Request</BaseButton>
+      </Section>
+    </BaseEmailWrapper>
+  );
+};
+
+export default AccessApprovalRequestUpdatedTemplate;
+
+AccessApprovalRequestUpdatedTemplate.PreviewProps = {
+  requesterFullName: "Abigail Williams",
+  requesterEmail: "[email protected]",
+  isTemporary: true,
+  secretPath: "/api/secrets",
+  environment: "Production",
+  siteUrl: "https://infisical.com",
+  projectName: "Example Project",
+  expiresIn: "1 day",
+  permissions: ["Read Secret", "Delete Project", "Create Dynamic Secret"],
+  editNote: "Too permissive, they only need 3 days",
+  editorEmail: "[email protected]",
+  editorFullName: "John Smith"
+} as AccessApprovalRequestUpdatedTemplateProps;
@@ -1,4 +1,5 @@
 export * from "./AccessApprovalRequestTemplate";
+export * from "./AccessApprovalRequestUpdatedTemplate";
 export * from "./EmailMfaTemplate";
 export * from "./EmailVerificationTemplate";
 export * from "./ExternalImportFailedTemplate";
@@ -8,6 +8,7 @@ import { logger } from "@app/lib/logger";

 import {
  AccessApprovalRequestTemplate,
+  AccessApprovalRequestUpdatedTemplate,
  EmailMfaTemplate,
  EmailVerificationTemplate,
  ExternalImportFailedTemplate,
@@ -54,6 +55,7 @@ export enum SmtpTemplates {
  EmailMfa = "emailMfa",
  UnlockAccount = "unlockAccount",
  AccessApprovalRequest = "accessApprovalRequest",
+  AccessApprovalRequestUpdated = "accessApprovalRequestUpdated",
  AccessSecretRequestBypassed = "accessSecretRequestBypassed",
  SecretApprovalRequestNeedsReview = "secretApprovalRequestNeedsReview",
  // HistoricalSecretList = "historicalSecretLeakIncident", not used anymore?
@@ -96,6 +98,7 @@ const EmailTemplateMap: Record<SmtpTemplates, React.FC<any>> = {
  [SmtpTemplates.SignupEmailVerification]: SignupEmailVerificationTemplate,
  [SmtpTemplates.EmailMfa]: EmailMfaTemplate,
  [SmtpTemplates.AccessApprovalRequest]: AccessApprovalRequestTemplate,
+  [SmtpTemplates.AccessApprovalRequestUpdated]: AccessApprovalRequestUpdatedTemplate,
  [SmtpTemplates.EmailVerification]: EmailVerificationTemplate,
  [SmtpTemplates.ExternalImportFailed]: ExternalImportFailedTemplate,
  [SmtpTemplates.ExternalImportStarted]: ExternalImportStartedTemplate,