Merge pull request #3997 from Infisical/fix-approval-requests-blocking-deletion

fix(approval-workflows): allow null committer on secret approval request and cascade delete on access request

backend/src/db/migrations/20250710153448_adjust-approval-request-user-cols.ts

@@ -0,0 +1,35 @@
+import { Knex } from "knex";
+
+import { TableName } from "@app/db/schemas";
+
+export async function up(knex: Knex): Promise<void> {
+  const hasCommitterCol = await knex.schema.hasColumn(TableName.SecretApprovalRequest, "committerUserId");
+
+  if (hasCommitterCol) {
+    await knex.schema.alterTable(TableName.SecretApprovalRequest, (tb) => {
+      tb.uuid("committerUserId").nullable().alter();
+    });
+  }
+
+  const hasRequesterCol = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "requestedByUserId");
+
+  if (hasRequesterCol) {
+    await knex.schema.alterTable(TableName.AccessApprovalRequest, (tb) => {
+      tb.dropForeign("requestedByUserId");
+      tb.foreign("requestedByUserId").references("id").inTable(TableName.Users).onDelete("CASCADE");
+    });
+  }
+}
+
+export async function down(knex: Knex): Promise<void> {
+  // can't undo committer nullable
+
+  const hasRequesterCol = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "requestedByUserId");
+
+  if (hasRequesterCol) {
+    await knex.schema.alterTable(TableName.AccessApprovalRequest, (tb) => {
+      tb.dropForeign("requestedByUserId");
+      tb.foreign("requestedByUserId").references("id").inTable(TableName.Users).onDelete("SET NULL");
+    });
+  }
+}

backend/src/db/schemas/access-approval-policies-approvers.ts

@@ -14,8 +14,8 @@ export const AccessApprovalPoliciesApproversSchema = z.object({
   updatedAt: z.date(),
   approverUserId: z.string().uuid().nullable().optional(),
   approverGroupId: z.string().uuid().nullable().optional(),
-  sequence: z.number().default(0).nullable().optional(),
-  approvalsRequired: z.number().default(1).nullable().optional()
+  sequence: z.number().default(1).nullable().optional(),
+  approvalsRequired: z.number().nullable().optional()
 });
 
 export type TAccessApprovalPoliciesApprovers = z.infer<typeof AccessApprovalPoliciesApproversSchema>;

backend/src/db/schemas/certificate-authorities.ts

@@ -12,8 +12,8 @@ export const CertificateAuthoritiesSchema = z.object({
   createdAt: z.date(),
   updatedAt: z.date(),
   projectId: z.string(),
-  enableDirectIssuance: z.boolean().default(true),
   status: z.string(),
+  enableDirectIssuance: z.boolean().default(true),
   name: z.string()
 });
 

backend/src/db/schemas/certificates.ts

@@ -25,8 +25,8 @@ export const CertificatesSchema = z.object({
   certificateTemplateId: z.string().uuid().nullable().optional(),
   keyUsages: z.string().array().nullable().optional(),
   extendedKeyUsages: z.string().array().nullable().optional(),
-  pkiSubscriberId: z.string().uuid().nullable().optional(),
-  projectId: z.string()
+  projectId: z.string(),
+  pkiSubscriberId: z.string().uuid().nullable().optional()
 });
 
 export type TCertificates = z.infer<typeof CertificatesSchema>;

backend/src/db/schemas/secret-approval-requests.ts

@@ -18,7 +18,7 @@ export const SecretApprovalRequestsSchema = z.object({
   createdAt: z.date(),
   updatedAt: z.date(),
   isReplicated: z.boolean().nullable().optional(),
-  committerUserId: z.string().uuid(),
+  committerUserId: z.string().uuid().nullable().optional(),
   statusChangedByUserId: z.string().uuid().nullable().optional(),
   bypassReason: z.string().nullable().optional()
 });

backend/src/ee/routes/v1/secret-approval-request-router.ts

@@ -58,7 +58,7 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
               deletedAt: z.date().nullish(),
               allowedSelfApprovals: z.boolean()
             }),
-            committerUser: approvalRequestUser,
+            committerUser: approvalRequestUser.nullish(),
             commits: z.object({ op: z.string(), secretId: z.string().nullable().optional() }).array(),
             environment: z.string(),
             reviewers: z.object({ userId: z.string(), status: z.string() }).array(),
@@ -308,7 +308,7 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
               }),
               environment: z.string(),
               statusChangedByUser: approvalRequestUser.optional(),
-              committerUser: approvalRequestUser,
+              committerUser: approvalRequestUser.nullish(),
               reviewers: approvalRequestUser.extend({ status: z.string(), comment: z.string().optional() }).array(),
               secretPath: z.string(),
               commits: secretRawSchema

backend/src/ee/services/audit-log/audit-log-types.ts

@@ -1711,7 +1711,7 @@ interface SecretApprovalReopened {
 interface SecretApprovalRequest {
   type: EventType.SECRET_APPROVAL_REQUEST;
   metadata: {
-    committedBy: string;
+    committedBy?: string | null;
     secretApprovalRequestSlug: string;
     secretApprovalRequestId: string;
     eventType: SecretApprovalEvent;

backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts

@@ -45,7 +45,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
         `${TableName.SecretApprovalRequest}.statusChangedByUserId`,
         `statusChangedByUser.id`
       )
-      .join<TUsers>(
+      .leftJoin<TUsers>(
         db(TableName.Users).as("committerUser"),
         `${TableName.SecretApprovalRequest}.committerUserId`,
         `committerUser.id`
@@ -173,13 +173,15 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
                 username: el.statusChangedByUserUsername
               }
             : undefined,
-          committerUser: {
-            userId: el.committerUserId,
-            email: el.committerUserEmail,
-            firstName: el.committerUserFirstName,
-            lastName: el.committerUserLastName,
-            username: el.committerUserUsername
-          },
+          committerUser: el.committerUserId
+            ? {
+                userId: el.committerUserId,
+                email: el.committerUserEmail,
+                firstName: el.committerUserFirstName,
+                lastName: el.committerUserLastName,
+                username: el.committerUserUsername
+              }
+            : null,
           policy: {
             id: el.policyId,
             name: el.policyName,
@@ -377,7 +379,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
           `${TableName.SecretApprovalPolicyBypasser}.bypasserGroupId`,
           `bypasserUserGroupMembership.groupId`
         )
-        .join<TUsers>(
+        .leftJoin<TUsers>(
           db(TableName.Users).as("committerUser"),
           `${TableName.SecretApprovalRequest}.committerUserId`,
           `committerUser.id`
@@ -488,13 +490,15 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
             enforcementLevel: el.policyEnforcementLevel,
             allowedSelfApprovals: el.policyAllowedSelfApprovals
           },
-          committerUser: {
-            userId: el.committerUserId,
-            email: el.committerUserEmail,
-            firstName: el.committerUserFirstName,
-            lastName: el.committerUserLastName,
-            username: el.committerUserUsername
-          }
+          committerUser: el.committerUserId
+            ? {
+                userId: el.committerUserId,
+                email: el.committerUserEmail,
+                firstName: el.committerUserFirstName,
+                lastName: el.committerUserLastName,
+                username: el.committerUserUsername
+              }
+            : null
         }),
         childrenMapper: [
           {
@@ -581,7 +585,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
           `${TableName.SecretApprovalPolicyBypasser}.bypasserGroupId`,
           `bypasserUserGroupMembership.groupId`
         )
-        .join<TUsers>(
+        .leftJoin<TUsers>(
           db(TableName.Users).as("committerUser"),
           `${TableName.SecretApprovalRequest}.committerUserId`,
           `committerUser.id`
@@ -693,13 +697,15 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
             enforcementLevel: el.policyEnforcementLevel,
             allowedSelfApprovals: el.policyAllowedSelfApprovals
           },
-          committerUser: {
-            userId: el.committerUserId,
-            email: el.committerUserEmail,
-            firstName: el.committerUserFirstName,
-            lastName: el.committerUserLastName,
-            username: el.committerUserUsername
-          }
+          committerUser: el.committerUserId
+            ? {
+                userId: el.committerUserId,
+                email: el.committerUserEmail,
+                firstName: el.committerUserFirstName,
+                lastName: el.committerUserLastName,
+                username: el.committerUserUsername
+              }
+            : null
         }),
         childrenMapper: [
           {

backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts

@@ -1320,7 +1320,7 @@ export const secretApprovalRequestServiceFactory = ({
     });
 
     const env = await projectEnvDAL.findOne({ id: policy.envId });
-    const user = await userDAL.findById(secretApprovalRequest.committerUserId);
+    const user = await userDAL.findById(actorId);
 
     await triggerWorkflowIntegrationNotification({
       input: {
@@ -1657,7 +1657,7 @@ export const secretApprovalRequestServiceFactory = ({
       return { ...doc, commits: approvalCommits };
     });
 
-    const user = await userDAL.findById(secretApprovalRequest.committerUserId);
+    const user = await userDAL.findById(actorId);
     const env = await projectEnvDAL.findOne({ id: policy.envId });
 
     await triggerWorkflowIntegrationNotification({

frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/SecretApprovalRequest.tsx

@@ -338,8 +338,14 @@ export const SecretApprovalRequest = () => {
                     </div>
                     <span className="text-xs leading-3 text-gray-500">
                       Opened {formatDistance(new Date(createdAt), new Date())} ago by{" "}
-                      {committerUser?.firstName || ""} {committerUser?.lastName || ""} (
-                      {committerUser?.email})
+                      {committerUser ? (
+                        <>
+                          {committerUser?.firstName || ""} {committerUser?.lastName || ""} (
+                          {committerUser?.email})
+                        </>
+                      ) : (
+                        <span className="text-gray-600">Deleted User</span>
+                      )}
                       {!isReviewed && status === "open" && " - Review required"}
                     </span>
                   </div>

frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx

@@ -250,10 +250,17 @@ export const SecretApprovalRequestChanges = ({
                 secretApprovalRequestDetails.isReplicated
               )}
             </div>
-            <span className="-mt-1 flex items-center space-x-2 text-xs text-gray-400">
-              By {secretApprovalRequestDetails?.committerUser?.firstName} (
-              {secretApprovalRequestDetails?.committerUser?.email})
-            </span>
+            <p className="-mt-1 text-xs text-gray-400">
+              By{" "}
+              {secretApprovalRequestDetails?.committerUser ? (
+                <>
+                  {secretApprovalRequestDetails?.committerUser?.firstName} (
+                  {secretApprovalRequestDetails?.committerUser?.email})
+                </>
+              ) : (
+                <span className="text-gray-500">Deleted User</span>
+              )}
+            </p>
           </div>
           {!hasMerged &&
             secretApprovalRequestDetails.status === "open" &&