Merge pull request #3878 from Infisical/fix-approval-policy-bypassing

Fix bypassing approval policies
This commit is contained in:
x032205
2025-06-30 13:37:28 -04:00
committed by GitHub
2 changed files with 39 additions and 38 deletions
@@ -350,6 +350,12 @@ export const accessApprovalRequestServiceFactory = ({
const canBypass = !policy.bypassers.length || policy.bypassers.some((bypasser) => bypasser.userId === actorId); const canBypass = !policy.bypassers.length || policy.bypassers.some((bypasser) => bypasser.userId === actorId);
const cannotBypassUnderSoftEnforcement = !(isSoftEnforcement && canBypass); const cannotBypassUnderSoftEnforcement = !(isSoftEnforcement && canBypass);
// Calculate break glass attempt before sequence checks
const isBreakGlassApprovalAttempt =
policy.enforcementLevel === EnforcementLevel.Soft &&
actorId === accessApprovalRequest.requestedByUserId &&
status === ApprovalStatus.APPROVED;
const isApprover = policy.approvers.find((approver) => approver.userId === actorId); const isApprover = policy.approvers.find((approver) => approver.userId === actorId);
// If user is (not an approver OR cant self approve) AND can't bypass policy // If user is (not an approver OR cant self approve) AND can't bypass policy
if ((!isApprover || (!policy.allowedSelfApprovals && isSelfApproval)) && cannotBypassUnderSoftEnforcement) { if ((!isApprover || (!policy.allowedSelfApprovals && isSelfApproval)) && cannotBypassUnderSoftEnforcement) {
@@ -409,15 +415,14 @@ export const accessApprovalRequestServiceFactory = ({
const isApproverOfTheSequence = policy.approvers.find( const isApproverOfTheSequence = policy.approvers.find(
(el) => el.sequence === presentSequence.step && el.userId === actorId (el) => el.sequence === presentSequence.step && el.userId === actorId
); );
if (!isApproverOfTheSequence) throw new BadRequestError({ message: "You are not reviewer in this step" });
// Only throw if actor is not the approver and not bypassing
if (!isApproverOfTheSequence && !isBreakGlassApprovalAttempt) {
throw new BadRequestError({ message: "You are not a reviewer in this step" });
}
} }
const reviewStatus = await accessApprovalRequestReviewerDAL.transaction(async (tx) => { const reviewStatus = await accessApprovalRequestReviewerDAL.transaction(async (tx) => {
const isBreakGlassApprovalAttempt =
policy.enforcementLevel === EnforcementLevel.Soft &&
actorId === accessApprovalRequest.requestedByUserId &&
status === ApprovalStatus.APPROVED;
let reviewForThisActorProcessing: { let reviewForThisActorProcessing: {
id: string; id: string;
requestId: string; requestId: string;
@@ -439,10 +439,7 @@ export const ReviewAccessRequestModal = ({
</div> </div>
) : ( ) : (
<> <>
{isSoftEnforcement && {isSoftEnforcement && request.isRequestedByCurrentUser && canBypass && (
request.isRequestedByCurrentUser &&
!(request.isApprover && request.isSelfApproveAllowed) &&
canBypass && (
<div className="mt-2 flex flex-col space-y-2"> <div className="mt-2 flex flex-col space-y-2">
<Checkbox <Checkbox
onCheckedChange={(checked) => setBypassApproval(checked === true)} onCheckedChange={(checked) => setBypassApproval(checked === true)}
@@ -451,8 +448,7 @@ export const ReviewAccessRequestModal = ({
className={twMerge("mr-2", bypassApproval ? "border-red/30 bg-red/10" : "")} className={twMerge("mr-2", bypassApproval ? "border-red/30 bg-red/10" : "")}
> >
<span className="text-xs text-red"> <span className="text-xs text-red">
Approve without waiting for requirements to be met (bypass policy Approve without waiting for requirements to be met (bypass policy protection)
protection)
</span> </span>
</Checkbox> </Checkbox>
{bypassApproval && ( {bypassApproval && (
@@ -460,7 +456,7 @@ export const ReviewAccessRequestModal = ({
label="Reason for bypass" label="Reason for bypass"
className="mt-2" className="mt-2"
isRequired isRequired
tooltipText="Enter a reason for bypassing the secret change policy" tooltipText="Enter a reason for bypassing the policy"
> >
<Input <Input
value={bypassReason} value={bypassReason}