Merge pull request #3184 from Infisical/feat/add-secret-approval-review-comment

feat: add secret approval review comment
This commit is contained in:
Maidul Islam
2025-03-05 12:24:59 -05:00
committed by GitHub
12 changed files with 294 additions and 47 deletions

View File

@@ -0,0 +1,19 @@
import { Knex } from "knex";
import { TableName } from "../schemas";
export async function up(knex: Knex): Promise<void> {
if (!(await knex.schema.hasColumn(TableName.SecretApprovalRequestReviewer, "comment"))) {
await knex.schema.alterTable(TableName.SecretApprovalRequestReviewer, (t) => {
t.string("comment");
});
}
}
export async function down(knex: Knex): Promise<void> {
if (await knex.schema.hasColumn(TableName.SecretApprovalRequestReviewer, "comment")) {
await knex.schema.alterTable(TableName.SecretApprovalRequestReviewer, (t) => {
t.dropColumn("comment");
});
}
}

View File

@@ -13,7 +13,8 @@ export const SecretApprovalRequestsReviewersSchema = z.object({
requestId: z.string().uuid(),
createdAt: z.date(),
updatedAt: z.date(),
reviewerUserId: z.string().uuid()
reviewerUserId: z.string().uuid(),
comment: z.string().nullable().optional()
});
export type TSecretApprovalRequestsReviewers = z.infer<typeof SecretApprovalRequestsReviewersSchema>;

View File

@@ -159,7 +159,8 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
id: z.string()
}),
body: z.object({
status: z.enum([ApprovalStatus.APPROVED, ApprovalStatus.REJECTED])
status: z.enum([ApprovalStatus.APPROVED, ApprovalStatus.REJECTED]),
comment: z.string().optional()
}),
response: {
200: z.object({
@@ -175,8 +176,25 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
actorAuthMethod: req.permission.authMethod,
actorOrgId: req.permission.orgId,
approvalId: req.params.id,
status: req.body.status
status: req.body.status,
comment: req.body.comment
});
await server.services.auditLog.createAuditLog({
...req.auditLogInfo,
orgId: req.permission.orgId,
projectId: review.projectId,
event: {
type: EventType.SECRET_APPROVAL_REQUEST_REVIEW,
metadata: {
secretApprovalRequestId: review.requestId,
reviewedBy: review.reviewerUserId,
status: review.status as ApprovalStatus,
comment: review.comment || ""
}
}
});
return { review };
}
});
@@ -267,7 +285,7 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
environment: z.string(),
statusChangedByUser: approvalRequestUser.optional(),
committerUser: approvalRequestUser,
reviewers: approvalRequestUser.extend({ status: z.string() }).array(),
reviewers: approvalRequestUser.extend({ status: z.string(), comment: z.string().optional() }).array(),
secretPath: z.string(),
commits: secretRawSchema
.omit({ _id: true, environment: true, workspace: true, type: true, version: true })

View File

@@ -22,6 +22,7 @@ import {
} from "@app/services/secret-sync/secret-sync-types";
import { KmipPermission } from "../kmip/kmip-enum";
import { ApprovalStatus } from "../secret-approval-request/secret-approval-request-types";
export type TListProjectAuditLogDTO = {
filter: {
@@ -165,6 +166,7 @@ export enum EventType {
SECRET_APPROVAL_REQUEST = "secret-approval-request",
SECRET_APPROVAL_CLOSED = "secret-approval-closed",
SECRET_APPROVAL_REOPENED = "secret-approval-reopened",
SECRET_APPROVAL_REQUEST_REVIEW = "secret-approval-request-review",
SIGN_SSH_KEY = "sign-ssh-key",
ISSUE_SSH_CREDS = "issue-ssh-creds",
CREATE_SSH_CA = "create-ssh-certificate-authority",
@@ -1314,6 +1316,16 @@ interface SecretApprovalRequest {
};
}
interface SecretApprovalRequestReview {
type: EventType.SECRET_APPROVAL_REQUEST_REVIEW;
metadata: {
secretApprovalRequestId: string;
reviewedBy: string;
status: ApprovalStatus;
comment: string;
};
}
interface SignSshKey {
type: EventType.SIGN_SSH_KEY;
metadata: {
@@ -2482,4 +2494,5 @@ export type Event =
| KmipOperationRevokeEvent
| KmipOperationLocateEvent
| KmipOperationRegisterEvent
| CreateSecretRequestEvent;
| CreateSecretRequestEvent
| SecretApprovalRequestReview;

View File

@@ -100,6 +100,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
tx.ref("lastName").withSchema("committerUser").as("committerUserLastName"),
tx.ref("reviewerUserId").withSchema(TableName.SecretApprovalRequestReviewer),
tx.ref("status").withSchema(TableName.SecretApprovalRequestReviewer).as("reviewerStatus"),
tx.ref("comment").withSchema(TableName.SecretApprovalRequestReviewer).as("reviewerComment"),
tx.ref("email").withSchema("secretApprovalReviewerUser").as("reviewerEmail"),
tx.ref("username").withSchema("secretApprovalReviewerUser").as("reviewerUsername"),
tx.ref("firstName").withSchema("secretApprovalReviewerUser").as("reviewerFirstName"),
@@ -162,8 +163,10 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
reviewerEmail: email,
reviewerLastName: lastName,
reviewerUsername: username,
reviewerFirstName: firstName
}) => (userId ? { userId, status, email, firstName, lastName, username } : undefined)
reviewerFirstName: firstName,
reviewerComment: comment
}) =>
userId ? { userId, status, email, firstName, lastName, username, comment: comment ?? "" } : undefined
},
{
key: "approverUserId",

View File

@@ -320,6 +320,7 @@ export const secretApprovalRequestServiceFactory = ({
approvalId,
actor,
status,
comment,
actorId,
actorAuthMethod,
actorOrgId
@@ -372,15 +373,18 @@ export const secretApprovalRequestServiceFactory = ({
return secretApprovalRequestReviewerDAL.create(
{
status,
comment,
requestId: secretApprovalRequest.id,
reviewerUserId: actorId
},
tx
);
}
return secretApprovalRequestReviewerDAL.updateById(review.id, { status }, tx);
return secretApprovalRequestReviewerDAL.updateById(review.id, { status, comment }, tx);
});
return reviewStatus;
return { ...reviewStatus, projectId: secretApprovalRequest.projectId };
};
const updateApprovalStatus = async ({

View File

@@ -80,6 +80,7 @@ export type TStatusChangeDTO = {
export type TReviewRequestDTO = {
approvalId: string;
status: ApprovalStatus;
comment?: string;
} & Omit<TProjectPermission, "projectId">;
export type TApprovalRequestCountDTO = TProjectPermission;

View File

@@ -122,6 +122,7 @@ export const eventToNameMap: { [K in EventType]: string } = {
"OIDC group membership mapping assigned user to groups",
[EventType.OIDC_GROUP_MEMBERSHIP_MAPPING_REMOVE_USER]:
"OIDC group membership mapping removed user from groups",
[EventType.SECRET_APPROVAL_REQUEST_REVIEW]: "Review Secret Approval Request",
[EventType.CREATE_KMIP_CLIENT]: "Create KMIP client",
[EventType.UPDATE_KMIP_CLIENT]: "Update KMIP client",
[EventType.DELETE_KMIP_CLIENT]: "Delete KMIP client",

View File

@@ -150,5 +150,6 @@ export enum EventType {
KMIP_OPERATION_ACTIVATE = "kmip-operation-activate",
KMIP_OPERATION_REVOKE = "kmip-operation-revoke",
KMIP_OPERATION_LOCATE = "kmip-operation-locate",
KMIP_OPERATION_REGISTER = "kmip-operation-register"
KMIP_OPERATION_REGISTER = "kmip-operation-register",
SECRET_APPROVAL_REQUEST_REVIEW = "secret-approval-request-review"
}

View File

@@ -13,9 +13,10 @@ export const useUpdateSecretApprovalReviewStatus = () => {
const queryClient = useQueryClient();
return useMutation<object, object, TUpdateSecretApprovalReviewStatusDTO>({
mutationFn: async ({ id, status }) => {
mutationFn: async ({ id, status, comment }) => {
const { data } = await apiRequest.post(`/api/v1/secret-approval-requests/${id}/review`, {
status
status,
comment
});
return data;
},

View File

@@ -44,6 +44,7 @@ export type TSecretApprovalRequest = {
reviewers: {
userId: string;
status: ApprovalStatus;
comment: string;
email: string;
firstName: string;
lastName: string;
@@ -114,6 +115,7 @@ export type TGetSecretApprovalRequestDetails = {
export type TUpdateSecretApprovalReviewStatusDTO = {
status: ApprovalStatus;
comment?: string;
id: string;
};

View File

@@ -1,19 +1,36 @@
import { ReactNode } from "react";
import { Controller, useForm } from "react-hook-form";
import {
faAngleDown,
faArrowLeft,
faCheck,
faCheckCircle,
faCircle,
faCodeBranch,
faComment,
faFolder,
faXmarkCircle
} from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { zodResolver } from "@hookform/resolvers/zod";
import { RadioGroup, RadioGroupIndicator, RadioGroupItem } from "@radix-ui/react-radio-group";
import { twMerge } from "tailwind-merge";
import z from "zod";
import { createNotification } from "@app/components/notifications";
import { Button, ContentLoader, EmptyState, IconButton, Tooltip } from "@app/components/v2";
import {
Button,
ContentLoader,
DropdownMenu,
DropdownMenuContent,
DropdownMenuTrigger,
EmptyState,
FormControl,
IconButton,
TextArea,
Tooltip
} from "@app/components/v2";
import { useUser } from "@app/context";
import { usePopUp } from "@app/hooks";
import {
useGetSecretApprovalRequestDetails,
useUpdateSecretApprovalReviewStatus
@@ -74,6 +91,13 @@ type Props = {
onGoBack: () => void;
};
const reviewFormSchema = z.object({
comment: z.string().trim().optional().default(""),
status: z.nativeEnum(ApprovalStatus)
});
type TReviewFormSchema = z.infer<typeof reviewFormSchema>;
export const SecretApprovalRequestChanges = ({
approvalRequestId,
onGoBack,
@@ -94,6 +118,16 @@ export const SecretApprovalRequestChanges = ({
variables
} = useUpdateSecretApprovalReviewStatus();
const { popUp, handlePopUpToggle } = usePopUp(["reviewChanges"] as const);
const {
control,
handleSubmit,
reset,
formState: { isSubmitting }
} = useForm<TReviewFormSchema>({
resolver: zodResolver(reviewFormSchema)
});
const isApproving = variables?.status === ApprovalStatus.APPROVED && isUpdatingRequestStatus;
const isRejecting = variables?.status === ApprovalStatus.REJECTED && isUpdatingRequestStatus;
@@ -101,23 +135,23 @@ export const SecretApprovalRequestChanges = ({
const canApprove = secretApprovalRequestDetails?.policy?.approvers?.some(
({ userId }) => userId === userSession.id
);
const reviewedUsers = secretApprovalRequestDetails?.reviewers?.reduce<
Record<string, ApprovalStatus>
Record<string, { status: ApprovalStatus; comment: string }>
>(
(prev, curr) => ({
...prev,
[curr.userId]: curr.status
[curr.userId]: { status: curr.status, comment: curr.comment }
}),
{}
);
const hasApproved = reviewedUsers?.[userSession.id] === ApprovalStatus.APPROVED;
const hasRejected = reviewedUsers?.[userSession.id] === ApprovalStatus.REJECTED;
const handleSecretApprovalStatusUpdate = async (status: ApprovalStatus) => {
const handleSecretApprovalStatusUpdate = async (status: ApprovalStatus, comment: string) => {
try {
await updateSecretApprovalRequestStatus({
id: approvalRequestId,
status
status,
comment
});
createNotification({
type: "success",
@@ -130,6 +164,16 @@ export const SecretApprovalRequestChanges = ({
text: "Failed to update the request status"
});
}
handlePopUpToggle("reviewChanges", false);
reset({
comment: "",
status: ApprovalStatus.APPROVED
});
};
const handleSubmitReview = (data: TReviewFormSchema) => {
handleSecretApprovalStatusUpdate(data.status, data.comment);
};
if (isSecretApprovalRequestLoading) {
@@ -150,7 +194,7 @@ export const SecretApprovalRequestChanges = ({
const isMergable =
secretApprovalRequestDetails?.policy?.approvals <=
secretApprovalRequestDetails?.policy?.approvers?.filter(
({ userId }) => reviewedUsers?.[userId] === ApprovalStatus.APPROVED
({ userId }) => reviewedUsers?.[userId]?.status === ApprovalStatus.APPROVED
).length;
const hasMerged = secretApprovalRequestDetails?.hasMerged;
@@ -202,27 +246,115 @@ export const SecretApprovalRequestChanges = ({
</div>
</div>
{!hasMerged && secretApprovalRequestDetails.status === "open" && (
<>
<Button
size="xs"
leftIcon={hasApproved && <FontAwesomeIcon icon={faCheck} />}
onClick={() => handleSecretApprovalStatusUpdate(ApprovalStatus.APPROVED)}
isLoading={isApproving}
isDisabled={isApproving || hasApproved || !canApprove}
>
{hasApproved ? "Approved" : "Approve"}
</Button>
<Button
size="xs"
colorSchema="danger"
leftIcon={hasRejected && <FontAwesomeIcon icon={faCheck} />}
onClick={() => handleSecretApprovalStatusUpdate(ApprovalStatus.REJECTED)}
isLoading={isRejecting}
isDisabled={isRejecting || hasRejected || !canApprove}
>
{hasRejected ? "Rejected" : "Reject"}
</Button>
</>
<DropdownMenu
open={popUp.reviewChanges.isOpen}
onOpenChange={(isOpen) => handlePopUpToggle("reviewChanges", isOpen)}
>
<DropdownMenuTrigger asChild>
<Button
variant="outline_bg"
rightIcon={<FontAwesomeIcon className="ml-2" icon={faAngleDown} />}
>
Review
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" asChild className="mt-3">
<form onSubmit={handleSubmit(handleSubmitReview)}>
<div className="flex w-[400px] flex-col space-y-2 p-5">
<div className="text-lg font-medium">Finish your review</div>
<Controller
control={control}
name="comment"
render={({ field, fieldState: { error } }) => (
<FormControl errorText={error?.message} isError={Boolean(error)}>
<TextArea
{...field}
placeholder="Leave a comment..."
reSize="none"
className="text-md mt-2 h-48 border border-mineshaft-600 bg-bunker-800"
/>
</FormControl>
)}
/>
<Controller
control={control}
name="status"
defaultValue={ApprovalStatus.APPROVED}
render={({ field, fieldState: { error } }) => (
<FormControl errorText={error?.message} isError={Boolean(error)}>
<RadioGroup
value={field.value}
onValueChange={field.onChange}
className="mb-4 space-y-2"
aria-label="Status"
>
<div className="flex items-center gap-2">
<RadioGroupItem
id="approve"
className="h-4 w-4 rounded-full border border-gray-300 text-primary focus:ring-2 focus:ring-mineshaft-500"
value={ApprovalStatus.APPROVED}
aria-labelledby="approve-label"
>
<RadioGroupIndicator className="flex h-full w-full items-center justify-center after:h-2 after:w-2 after:rounded-full after:bg-current" />
</RadioGroupItem>
<span
id="approve-label"
className="cursor-pointer"
onClick={() => field.onChange(ApprovalStatus.APPROVED)}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
field.onChange(ApprovalStatus.APPROVED);
}
}}
tabIndex={0}
role="button"
>
Approve
</span>
</div>
<div className="flex items-center gap-2">
<RadioGroupItem
id="reject"
className="h-4 w-4 rounded-full border border-gray-300 text-primary focus:ring-2 focus:ring-mineshaft-500"
value={ApprovalStatus.REJECTED}
aria-labelledby="reject-label"
>
<RadioGroupIndicator className="flex h-full w-full items-center justify-center after:h-2 after:w-2 after:rounded-full after:bg-current" />
</RadioGroupItem>
<span
id="reject-label"
className="cursor-pointer"
onClick={() => field.onChange(ApprovalStatus.REJECTED)}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
field.onChange(ApprovalStatus.REJECTED);
}
}}
tabIndex={0}
role="button"
>
Reject
</span>
</div>
</RadioGroup>
</FormControl>
)}
/>
<div className="flex justify-end">
<Button
type="submit"
isLoading={isApproving || isRejecting || isSubmitting}
variant="outline_bg"
>
Submit Review
</Button>
</div>
</div>
</form>
</DropdownMenuContent>
</DropdownMenu>
)}
</div>
<div className="flex flex-col space-y-4">
@@ -240,7 +372,40 @@ export const SecretApprovalRequestChanges = ({
)
)}
</div>
<div className="mt-8 flex items-center space-x-6 rounded-lg bg-mineshaft-800 px-5 py-6">
<div className="mt-4 flex flex-col items-center rounded-lg">
{secretApprovalRequestDetails?.policy?.approvers
.filter((requiredApprover) => reviewedUsers?.[requiredApprover.userId])
.map((requiredApprover) => {
const reviewer = reviewedUsers?.[requiredApprover.userId];
return (
<div
className="mb-4 flex w-full flex-col rounded-md bg-mineshaft-800 p-6"
key={`required-approver-${requiredApprover.userId}`}
>
<div>
<span className="ml-1">
{`${requiredApprover.firstName || ""} ${requiredApprover.lastName || ""}`} (
{requiredApprover?.email}) has{" "}
</span>
<span
className={`${reviewer?.status === ApprovalStatus.APPROVED ? "text-green-500" : "text-red-500"}`}
>
{reviewer?.status === ApprovalStatus.APPROVED ? "approved" : "rejected"}
</span>{" "}
the request.
</div>
{reviewer?.comment && (
<FormControl label="Comment" className="mb-0 mt-2">
<TextArea value={reviewer.comment} isDisabled reSize="none">
{reviewer?.comment && reviewer.comment}
</TextArea>
</FormControl>
)}
</div>
);
})}
</div>
<div className="flex items-center space-x-6 rounded-lg bg-mineshaft-800 px-5 py-6">
<SecretApprovalRequestAction
canApprove={canApprove}
approvalRequestId={secretApprovalRequestDetails.id}
@@ -258,7 +423,7 @@ export const SecretApprovalRequestChanges = ({
<div className="text-sm text-bunker-300">Reviewers</div>
<div className="mt-2 flex flex-col space-y-2 text-sm">
{secretApprovalRequestDetails?.policy?.approvers.map((requiredApprover) => {
const status = reviewedUsers?.[requiredApprover.userId];
const reviewer = reviewedUsers?.[requiredApprover.userId];
return (
<div
className="flex flex-nowrap items-center space-x-2 rounded bg-mineshaft-800 px-2 py-1"
@@ -275,8 +440,17 @@ export const SecretApprovalRequestChanges = ({
<span className="text-red">*</span>
</div>
<div>
<Tooltip content={status || ApprovalStatus.PENDING}>
{getReviewedStatusSymbol(status)}
{reviewer?.comment && (
<Tooltip content={reviewer.comment}>
<FontAwesomeIcon
icon={faComment}
size="xs"
className="mr-1 text-mineshaft-300"
/>
</Tooltip>
)}
<Tooltip content={reviewer?.status || ApprovalStatus.PENDING}>
{getReviewedStatusSymbol(reviewer?.status)}
</Tooltip>
</div>
</div>
@@ -290,7 +464,7 @@ export const SecretApprovalRequestChanges = ({
)
)
.map((reviewer) => {
const status = reviewedUsers?.[reviewer.userId];
const status = reviewedUsers?.[reviewer.userId].status;
return (
<div
className="flex flex-nowrap items-center space-x-2 rounded bg-mineshaft-800 px-2 py-1"
@@ -303,6 +477,15 @@ export const SecretApprovalRequestChanges = ({
<span className="text-red">*</span>
</div>
<div>
{reviewer.comment && (
<Tooltip content={reviewer.comment}>
<FontAwesomeIcon
icon={faComment}
size="xs"
className="mr-1 text-mineshaft-300"
/>
</Tooltip>
)}
<Tooltip content={status || ApprovalStatus.PENDING}>
{getReviewedStatusSymbol(status)}
</Tooltip>