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 010f8b2e7..d90f28184 100644 --- a/backend/src/ee/routes/v1/access-approval-request-router.ts +++ b/backend/src/ee/routes/v1/access-approval-request-router.ts @@ -155,7 +155,6 @@ export const registerAccessApprovalRequestRouter = async (server: FastifyZodProv }), body: z.object({ status: z.enum([ApprovalStatus.APPROVED, ApprovalStatus.REJECTED]), - envName: z.string().optional(), // For logging bypassReason: z.string().min(10).max(1000).optional() }), response: { @@ -173,7 +172,6 @@ export const registerAccessApprovalRequestRouter = async (server: FastifyZodProv actorAuthMethod: req.permission.authMethod, requestId: req.params.requestId, status: req.body.status, - envName: req.body.envName, bypassReason: req.body.bypassReason }); 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 f7ed787ac..017356a5d 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 @@ -326,7 +326,6 @@ export const accessApprovalRequestServiceFactory = ({ actorId, actorAuthMethod, actorOrgId, - envName, bypassReason }: TReviewAccessRequestDTO) => { const accessApprovalRequest = await accessApprovalRequestDAL.findById(requestId); @@ -334,7 +333,7 @@ export const accessApprovalRequestServiceFactory = ({ throw new NotFoundError({ message: `Secret approval request with ID '${requestId}' not found` }); } - const { policy } = accessApprovalRequest; + const { policy, environment } = accessApprovalRequest; if (policy.deletedAt) { throw new BadRequestError({ message: "The policy associated with this access request has been deleted." @@ -354,14 +353,15 @@ export const accessApprovalRequestServiceFactory = ({ throw new ForbiddenRequestError({ message: "You are not a member of this project" }); } - if ( - !policy.allowedSelfApprovals && - actorId === accessApprovalRequest.requestedByUserId && - !( - policy.enforcementLevel === EnforcementLevel.Soft && - permission.can(ProjectPermissionApprovalActions.AllowAccessBypass, ProjectPermissionSub.SecretApproval) - ) - ) { + const isSelfApproval = actorId === accessApprovalRequest.requestedByUserId; + const isSoftEnforcement = policy.enforcementLevel === EnforcementLevel.Soft; + const canBypassApproval = permission.can( + ProjectPermissionApprovalActions.AllowAccessBypass, + ProjectPermissionSub.SecretApproval + ); + const cannotBypassUnderSoftEnforcement = !(isSoftEnforcement && canBypassApproval); + + if (!policy.allowedSelfApprovals && isSelfApproval && cannotBypassUnderSoftEnforcement) { throw new BadRequestError({ message: "Failed to review access approval request. Users are not authorized to review their own request." }); @@ -508,7 +508,7 @@ export const accessApprovalRequestServiceFactory = ({ requesterEmail: actingUser.email, bypassReason: bypassReason || "No reason provided", secretPath: policy.secretPath || "/", - environment: envName || "Unknown", + environment, approvalUrl: `${cfg.SITE_URL}/secret-manager/${project.id}/approval`, requestType: "access" }, diff --git a/frontend/src/hooks/api/accessApproval/mutation.tsx b/frontend/src/hooks/api/accessApproval/mutation.tsx index 9ba895311..c0da7af23 100644 --- a/frontend/src/hooks/api/accessApproval/mutation.tsx +++ b/frontend/src/hooks/api/accessApproval/mutation.tsx @@ -129,30 +129,27 @@ export const useReviewAccessRequest = () => { requestId: string; status: "approved" | "rejected"; projectSlug: string; - envName?: string; envSlug?: string; requestedBy?: string; bypassReason?: string; } >({ - mutationFn: async ({ requestId, status, envName, bypassReason }) => { + mutationFn: async ({ requestId, status, bypassReason }) => { const { data } = await apiRequest.post( `/api/v1/access-approvals/requests/${requestId}/review`, { status, - envName, bypassReason } ); return data; }, - onSuccess: (_, { projectSlug, envSlug, requestedBy, envName, bypassReason }) => { + onSuccess: (_, { projectSlug, envSlug, requestedBy, bypassReason }) => { queryClient.invalidateQueries({ queryKey: accessApprovalKeys.getAccessApprovalRequests( projectSlug, envSlug, requestedBy, - envName, bypassReason ) }); diff --git a/frontend/src/hooks/api/accessApproval/queries.tsx b/frontend/src/hooks/api/accessApproval/queries.tsx index 0f666af7a..6370f4a59 100644 --- a/frontend/src/hooks/api/accessApproval/queries.tsx +++ b/frontend/src/hooks/api/accessApproval/queries.tsx @@ -23,13 +23,8 @@ export const accessApprovalKeys = { projectSlug: string, envSlug?: string, requestedBy?: string, - envName?: string, bypassReason?: string - ) => - [ - { projectSlug, envSlug, requestedBy, envName, bypassReason }, - "access-approvals-requests" - ] as const, + ) => [{ projectSlug, envSlug, requestedBy, bypassReason }, "access-approvals-requests"] as const, getAccessApprovalRequestCount: (projectSlug: string) => [{ projectSlug }, "access-approval-request-count"] as const }; diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/AccessApprovalRequest.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/AccessApprovalRequest.tsx index bf89accd0..7d5e1540a 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/AccessApprovalRequest.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/AccessApprovalRequest.tsx @@ -1,6 +1,6 @@ /* eslint-disable no-nested-ternary */ /* eslint-disable react/jsx-no-useless-fragment */ -import { useMemo, useState } from "react"; +import { useCallback, useMemo, useState } from "react"; import { faCheck, faCheckCircle, @@ -150,56 +150,105 @@ export const AccessApprovalRequest = ({ return requests; }, [requests, statusFilter, requestedByFilter, envFilter]); - const generateRequestDetails = (request: TAccessApprovalRequest) => { - const isReviewedByUser = request.reviewers.findIndex(({ member }) => member === user.id) !== -1; - const isRejectedByAnyone = request.reviewers.some( - ({ status }) => status === ApprovalStatus.REJECTED - ); - const isApprover = request.policy.approvers.indexOf(user.id || "") !== -1; - const isAccepted = request.isApproved; - const isSoftEnforcement = request.policy.enforcementLevel === EnforcementLevel.Soft; - const isRequestedByCurrentUser = request.requestedByUserId === user.id; - const isSelfApproveAllowed = request.policy.allowedSelfApprovals; - const userReviewStatus = request.reviewers.find(({ member }) => member === user.id)?.status; + const generateRequestDetails = useCallback( + (request: TAccessApprovalRequest) => { + const isReviewedByUser = + request.reviewers.findIndex(({ member }) => member === user.id) !== -1; + const isRejectedByAnyone = request.reviewers.some( + ({ status }) => status === ApprovalStatus.REJECTED + ); + const isApprover = request.policy.approvers.indexOf(user.id || "") !== -1; + const isAccepted = request.isApproved; + const isSoftEnforcement = request.policy.enforcementLevel === EnforcementLevel.Soft; + const isRequestedByCurrentUser = request.requestedByUserId === user.id; + const isSelfApproveAllowed = request.policy.allowedSelfApprovals; + const userReviewStatus = request.reviewers.find(({ member }) => member === user.id)?.status; - let displayData: { label: string; type: "primary" | "danger" | "success" } = { - label: "", - type: "primary" - }; - - const isExpired = - request.privilege && - request.isApproved && - new Date() > new Date(request.privilege.temporaryAccessEndTime || ("" as string)); - - if (isExpired) displayData = { label: "Access Expired", type: "danger" }; - else if (isAccepted) displayData = { label: "Access Granted", type: "success" }; - else if (isRejectedByAnyone) displayData = { label: "Rejected", type: "danger" }; - else if (userReviewStatus === ApprovalStatus.APPROVED) { - displayData = { - label: `Pending ${request.policy.approvals - request.reviewers.length} review${ - request.policy.approvals - request.reviewers.length > 1 ? "s" : "" - }`, - type: "primary" - }; - } else if (!isReviewedByUser) - displayData = { - label: "Review Required", + let displayData: { label: string; type: "primary" | "danger" | "success" } = { + label: "", type: "primary" }; - return { - displayData, - isReviewedByUser, - isRejectedByAnyone, - isApprover, - userReviewStatus, - isAccepted, - isSoftEnforcement, - isRequestedByCurrentUser, - isSelfApproveAllowed - }; - }; + const isExpired = + request.privilege && + request.isApproved && + new Date() > new Date(request.privilege.temporaryAccessEndTime || ("" as string)); + + if (isExpired) displayData = { label: "Access Expired", type: "danger" }; + else if (isAccepted) displayData = { label: "Access Granted", type: "success" }; + else if (isRejectedByAnyone) displayData = { label: "Rejected", type: "danger" }; + else if (userReviewStatus === ApprovalStatus.APPROVED) { + displayData = { + label: `Pending ${request.policy.approvals - request.reviewers.length} review${ + request.policy.approvals - request.reviewers.length > 1 ? "s" : "" + }`, + type: "primary" + }; + } else if (!isReviewedByUser) + displayData = { + label: "Review Required", + type: "primary" + }; + + return { + displayData, + isReviewedByUser, + isRejectedByAnyone, + isApprover, + userReviewStatus, + isAccepted, + isSoftEnforcement, + isRequestedByCurrentUser, + isSelfApproveAllowed + }; + }, + [user] + ); + + const handleSelectRequest = useCallback( + (request: TAccessApprovalRequest) => { + const details = generateRequestDetails(request); + + // Whether the request has already been approved / rejected / reviewed + const isInactive = + details.isAccepted || details.isReviewedByUser || details.isRejectedByAnyone; + + // Whether the current user can bypass policy + const canBypass = + details.isSoftEnforcement && + details.isRequestedByCurrentUser && + canBypassApprovalPermission; + + // Whether the current user can approve + const canApprove = + details.isApprover && (!details.isRequestedByCurrentUser || details.isSelfApproveAllowed); + + if (isInactive || (!canApprove && !canBypass)) return; + + if (membersGroupById?.[request.requestedByUserId].user || details.isRequestedByCurrentUser) { + setSelectedRequest({ + ...request, + user: + details.isRequestedByCurrentUser || !membersGroupById?.[request.requestedByUserId].user + ? user + : membersGroupById?.[request.requestedByUserId].user, + isRequestedByCurrentUser: details.isRequestedByCurrentUser, + isSelfApproveAllowed: details.isSelfApproveAllowed, + isApprover: details.isApprover + }); + } + + handlePopUpOpen("reviewRequest"); + }, + [ + generateRequestDetails, + canBypassApprovalPermission, + membersGroupById, + user, + setSelectedRequest, + handlePopUpOpen + ] + ); return (
@@ -351,84 +400,10 @@ export const AccessApprovalRequest = ({ className="flex w-full cursor-pointer px-8 py-4 hover:bg-mineshaft-700 aria-disabled:opacity-80" role="button" tabIndex={0} - onClick={() => { - // Whether the request has already been approved / rejected / reviewed - const isInactive = - details.isAccepted || - details.isReviewedByUser || - details.isRejectedByAnyone; - - // Whether the current user can bypass policy - const canBypass = - details.isSoftEnforcement && - details.isRequestedByCurrentUser && - canBypassApprovalPermission; - - // Whether the current user can approve - const canApprove = - details.isApprover && - (!details.isRequestedByCurrentUser || details.isSelfApproveAllowed); - - if (isInactive || (!canApprove && !canBypass)) return; - - if ( - membersGroupById?.[request.requestedByUserId].user || - details.isRequestedByCurrentUser - ) { - setSelectedRequest({ - ...request, - user: - details.isRequestedByCurrentUser || - !membersGroupById?.[request.requestedByUserId].user - ? user - : membersGroupById?.[request.requestedByUserId].user, - isRequestedByCurrentUser: details.isRequestedByCurrentUser, - isSelfApproveAllowed: details.isSelfApproveAllowed, - isApprover: details.isApprover - }); - } - - handlePopUpOpen("reviewRequest"); - }} + onClick={() => handleSelectRequest(request)} onKeyDown={(evt) => { - // Whether the request has already been approved / rejected / reviewed - const isInactive = - details.isAccepted || - details.isReviewedByUser || - details.isRejectedByAnyone; - - // Whether the current user can bypass policy - const canBypass = - details.isSoftEnforcement && - details.isRequestedByCurrentUser && - canBypassApprovalPermission; - - // Whether the current user can approve - const canApprove = - details.isApprover && - (!details.isRequestedByCurrentUser || details.isSelfApproveAllowed); - - if (isInactive || (!canApprove && !canBypass)) return; - if (evt.key === "Enter") { - if ( - membersGroupById?.[request.requestedByUserId].user || - details.isRequestedByCurrentUser - ) { - setSelectedRequest({ - ...request, - user: - details.isRequestedByCurrentUser || - !membersGroupById?.[request.requestedByUserId].user - ? user - : membersGroupById?.[request.requestedByUserId].user, - isRequestedByCurrentUser: details.isRequestedByCurrentUser, - isSelfApproveAllowed: details.isSelfApproveAllowed, - isApprover: details.isApprover - }); - } - - handlePopUpOpen("reviewRequest"); + handleSelectRequest(request); } }} > 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 f05d26c35..0b9f84008 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 @@ -104,7 +104,6 @@ export const ReviewAccessRequestModal = ({ requestId: request.id, status, projectSlug, - envName: accessDetails.env, envSlug: selectedEnvSlug, requestedBy: selectedRequester, bypassReason: bypassApproval ? bypassReason : undefined @@ -129,7 +128,6 @@ export const ReviewAccessRequestModal = ({ bypassReason, reviewAccessRequest, request, - accessDetails.env, selectedEnvSlug, selectedRequester, onOpenChange