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 20e954d44..f7ed787ac 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 @@ -386,7 +386,21 @@ export const accessApprovalRequestServiceFactory = ({ } const reviewStatus = await accessApprovalRequestReviewerDAL.transaction(async (tx) => { - const review = await accessApprovalRequestReviewerDAL.findOne( + const isBreakGlassApprovalAttempt = + policy.enforcementLevel === EnforcementLevel.Soft && + actorId === accessApprovalRequest.requestedByUserId && + status === ApprovalStatus.APPROVED; + + let reviewForThisActorProcessing: { + id: string; + requestId: string; + reviewerUserId: string; + status: string; + createdAt: Date; + updatedAt: Date; + }; + + const existingReviewByActorInTx = await accessApprovalRequestReviewerDAL.findOne( { requestId: accessApprovalRequest.id, reviewerUserId: actorId @@ -394,69 +408,82 @@ export const accessApprovalRequestServiceFactory = ({ tx ); - if (review) { - throw new BadRequestError({ message: "You have already reviewed this request" }); - } - - const newReview = await accessApprovalRequestReviewerDAL.create( - { - status, - requestId: accessApprovalRequest.id, - reviewerUserId: actorId - }, - tx - ); - - const allReviews = [...existingReviews, newReview]; - const approvedReviews = allReviews.filter((r) => r.status === ApprovalStatus.APPROVED); - - if (status === ApprovalStatus.APPROVED && approvedReviews.length >= policy.approvals) { - if (accessApprovalRequest.isTemporary && !accessApprovalRequest.temporaryRange) { - throw new BadRequestError({ message: "Temporary range is required for temporary access" }); - } - - let privilegeId: string | null = null; - - if (!accessApprovalRequest.isTemporary && !accessApprovalRequest.temporaryRange) { - // Permanent access - const privilege = await additionalPrivilegeDAL.create( - { - userId: accessApprovalRequest.requestedByUserId, - projectId: accessApprovalRequest.projectId, - slug: `requested-privilege-${slugify(alphaNumericNanoId(12))}`, - permissions: JSON.stringify(accessApprovalRequest.permissions) - }, - tx - ); - privilegeId = privilege.id; + // Check if review exists for actor + if (existingReviewByActorInTx) { + // Check if breakglass re-approval + if (isBreakGlassApprovalAttempt && existingReviewByActorInTx.status === ApprovalStatus.APPROVED) { + reviewForThisActorProcessing = existingReviewByActorInTx; } else { - // Temporary access - const relativeTempAllocatedTimeInMs = ms(accessApprovalRequest.temporaryRange!); - const startTime = new Date(); - - const privilege = await additionalPrivilegeDAL.create( - { - userId: accessApprovalRequest.requestedByUserId, - projectId: accessApprovalRequest.projectId, - slug: `requested-privilege-${slugify(alphaNumericNanoId(12))}`, - permissions: JSON.stringify(accessApprovalRequest.permissions), - isTemporary: true, // Explicitly set to true for the privilege - temporaryMode: ProjectUserAdditionalPrivilegeTemporaryMode.Relative, - temporaryRange: accessApprovalRequest.temporaryRange!, - temporaryAccessStartTime: startTime, - temporaryAccessEndTime: new Date(startTime.getTime() + relativeTempAllocatedTimeInMs) - }, - tx - ); - privilegeId = privilege.id; + throw new BadRequestError({ message: "You have already reviewed this request" }); } - await accessApprovalRequestDAL.updateById(accessApprovalRequest.id, { privilegeId }, tx); + } else { + reviewForThisActorProcessing = await accessApprovalRequestReviewerDAL.create( + { + status, + requestId: accessApprovalRequest.id, + reviewerUserId: actorId + }, + tx + ); } - const isSoftEnforcement = policy.enforcementLevel === EnforcementLevel.Soft; - const wasSelfRequestAndReview = actorId === accessApprovalRequest.requestedByUserId; + const otherReviews = existingReviews.filter((er) => er.reviewerUserId !== actorId); + const allUniqueReviews = [...otherReviews, reviewForThisActorProcessing]; - if (isSoftEnforcement && wasSelfRequestAndReview && status === ApprovalStatus.APPROVED) { + const approvedReviews = allUniqueReviews.filter((r) => r.status === ApprovalStatus.APPROVED); + const meetsStandardApprovalThreshold = approvedReviews.length >= policy.approvals; + + if ( + reviewForThisActorProcessing.status === ApprovalStatus.APPROVED && + (meetsStandardApprovalThreshold || isBreakGlassApprovalAttempt) + ) { + const currentRequestState = await accessApprovalRequestDAL.findById(accessApprovalRequest.id, tx); + let privilegeIdToSet = currentRequestState?.privilegeId || null; + + if (!privilegeIdToSet) { + if (accessApprovalRequest.isTemporary && !accessApprovalRequest.temporaryRange) { + throw new BadRequestError({ message: "Temporary range is required for temporary access" }); + } + + if (!accessApprovalRequest.isTemporary && !accessApprovalRequest.temporaryRange) { + // Permanent access + const privilege = await additionalPrivilegeDAL.create( + { + userId: accessApprovalRequest.requestedByUserId, + projectId: accessApprovalRequest.projectId, + slug: `requested-privilege-${slugify(alphaNumericNanoId(12))}`, + permissions: JSON.stringify(accessApprovalRequest.permissions) + }, + tx + ); + privilegeIdToSet = privilege.id; + } else { + // Temporary access + const relativeTempAllocatedTimeInMs = ms(accessApprovalRequest.temporaryRange!); + const startTime = new Date(); + + const privilege = await additionalPrivilegeDAL.create( + { + userId: accessApprovalRequest.requestedByUserId, + projectId: accessApprovalRequest.projectId, + slug: `requested-privilege-${slugify(alphaNumericNanoId(12))}`, + permissions: JSON.stringify(accessApprovalRequest.permissions), + isTemporary: true, // Explicitly set to true for the privilege + temporaryMode: ProjectUserAdditionalPrivilegeTemporaryMode.Relative, + temporaryRange: accessApprovalRequest.temporaryRange!, + temporaryAccessStartTime: startTime, + temporaryAccessEndTime: new Date(startTime.getTime() + relativeTempAllocatedTimeInMs) + }, + tx + ); + privilegeIdToSet = privilege.id; + } + await accessApprovalRequestDAL.updateById(accessApprovalRequest.id, { privilegeId: privilegeIdToSet }, tx); + } + } + + // Send notification if this was a breakglass approval + if (isBreakGlassApprovalAttempt) { const cfg = getConfig(); const actingUser = await userDAL.findById(actorId, tx); @@ -491,7 +518,7 @@ export const accessApprovalRequestServiceFactory = ({ } } } - return newReview; + return reviewForThisActorProcessing; }); return reviewStatus; 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 fd754b14c..bf89accd0 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/AccessApprovalRequest.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/AccessApprovalRequest.tsx @@ -86,6 +86,7 @@ export const AccessApprovalRequest = ({ | (TAccessApprovalRequest & { user: { firstName?: string; lastName?: string; email?: string } | null; isRequestedByCurrentUser: boolean; + isSelfApproveAllowed: boolean; isApprover: boolean; }) | null @@ -351,18 +352,24 @@ export const AccessApprovalRequest = ({ role="button" tabIndex={0} onClick={() => { - if ( + // Whether the request has already been approved / rejected / reviewed + const isInactive = details.isAccepted || details.isReviewedByUser || - details.isRejectedByAnyone || - (!details.isApprover && - !( - details.isSoftEnforcement && - details.isRequestedByCurrentUser && - canBypassApprovalPermission - )) - ) - return; + 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 || @@ -376,6 +383,7 @@ export const AccessApprovalRequest = ({ ? user : membersGroupById?.[request.requestedByUserId].user, isRequestedByCurrentUser: details.isRequestedByCurrentUser, + isSelfApproveAllowed: details.isSelfApproveAllowed, isApprover: details.isApprover }); } @@ -383,18 +391,24 @@ export const AccessApprovalRequest = ({ handlePopUpOpen("reviewRequest"); }} onKeyDown={(evt) => { - if ( + // Whether the request has already been approved / rejected / reviewed + const isInactive = details.isAccepted || details.isReviewedByUser || - details.isRejectedByAnyone || - (!details.isApprover && - !( - details.isSoftEnforcement && - details.isRequestedByCurrentUser && - canBypassApprovalPermission - )) - ) - return; + 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 ( @@ -409,6 +423,7 @@ export const AccessApprovalRequest = ({ ? user : membersGroupById?.[request.requestedByUserId].user, isRequestedByCurrentUser: details.isRequestedByCurrentUser, + isSelfApproveAllowed: details.isSelfApproveAllowed, isApprover: details.isApprover }); } 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 5aaa0ea56..f05d26c35 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 @@ -26,6 +26,7 @@ export const ReviewAccessRequestModal = ({ request: TAccessApprovalRequest & { user: { firstName?: string; lastName?: string; email?: string } | null; isRequestedByCurrentUser: boolean; + isSelfApproveAllowed: boolean; isApprover: boolean; }; projectSlug: string; @@ -155,7 +156,6 @@ export const ReviewAccessRequestModal = ({ )}{" "} is requesting access to the following resource: -
Requested path: @@ -179,16 +179,16 @@ export const ReviewAccessRequestModal = ({
)}
-
- {isSoftEnforcement && request.isRequestedByCurrentUser && - !request.isApprover && + !(request.isApprover && request.isSelfApproveAllowed) && canBypassApprovalPermission && (