diff --git a/backend/src/db/migrations/20250530152721_add-access-approval-request-deleted-at.ts b/backend/src/db/migrations/20250530152721_add-access-approval-request-deleted-at.ts new file mode 100644 index 000000000..771df8cf8 --- /dev/null +++ b/backend/src/db/migrations/20250530152721_add-access-approval-request-deleted-at.ts @@ -0,0 +1,61 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + const hasPrivilegeDeletedAtColumn = await knex.schema.hasColumn( + TableName.AccessApprovalRequest, + "privilegeDeletedAt" + ); + const hasStatusColumn = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "status"); + + if (!hasPrivilegeDeletedAtColumn) { + await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => { + t.timestamp("privilegeDeletedAt").nullable(); + }); + } + + if (!hasStatusColumn) { + await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => { + t.string("status").defaultTo("pending").notNullable(); + }); + + // Update existing rows based on business logic + // If privilegeId is not null, set status to "approved" + await knex(TableName.AccessApprovalRequest).whereNotNull("privilegeId").update({ status: "approved" }); + + // If privilegeId is null and there's a rejected reviewer, set to "rejected" + const rejectedRequestIds = await knex(TableName.AccessApprovalRequestReviewer) + .select("requestId") + .where("status", "rejected") + .distinct() + .pluck("requestId"); + + if (rejectedRequestIds.length > 0) { + await knex(TableName.AccessApprovalRequest) + .whereNull("privilegeId") + .whereIn("id", rejectedRequestIds) + .update({ status: "rejected" }); + } + } +} + +export async function down(knex: Knex): Promise { + const hasPrivilegeDeletedAtColumn = await knex.schema.hasColumn( + TableName.AccessApprovalRequest, + "privilegeDeletedAt" + ); + const hasStatusColumn = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "status"); + + if (hasPrivilegeDeletedAtColumn) { + await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => { + t.dropColumn("privilegeDeletedAt"); + }); + } + + if (hasStatusColumn) { + await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => { + t.dropColumn("status"); + }); + } +} diff --git a/backend/src/db/schemas/access-approval-requests.ts b/backend/src/db/schemas/access-approval-requests.ts index bfe990b3a..6a6f09148 100644 --- a/backend/src/db/schemas/access-approval-requests.ts +++ b/backend/src/db/schemas/access-approval-requests.ts @@ -18,7 +18,9 @@ export const AccessApprovalRequestsSchema = z.object({ createdAt: z.date(), updatedAt: z.date(), requestedByUserId: z.string().uuid(), - note: z.string().nullable().optional() + note: z.string().nullable().optional(), + privilegeDeletedAt: z.date().nullable().optional(), + status: z.string().default("pending") }); export type TAccessApprovalRequests = z.infer; diff --git a/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts b/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts index e2075af0a..b5be58382 100644 --- a/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts +++ b/backend/src/ee/services/access-approval-request/access-approval-request-dal.ts @@ -145,7 +145,7 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { } : null, - isApproved: !!doc.policyDeletedAt || !!doc.privilegeId + isApproved: !!doc.policyDeletedAt || !!doc.privilegeId || doc.status !== ApprovalStatus.PENDING }), childrenMapper: [ { @@ -392,14 +392,20 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => { ] }); - // an approval is pending if there is no reviewer rejections and no privilege ID is set + // an approval is pending if there is no reviewer rejections, no privilege ID is set and the number of approvals is less than the number of approvals required const pendingApprovals = formattedRequests.filter( - (req) => !req.privilegeId && !req.reviewers.some((r) => r.status === ApprovalStatus.REJECTED) + (req) => + !req.privilegeId && + !req.reviewers.some((r) => r.status === ApprovalStatus.REJECTED) && + req.status === ApprovalStatus.PENDING ); - // an approval is finalized if there are any rejections or a privilege ID is set + // an approval is finalized if there are any rejections, a privilege ID is set or the number of approvals is equal to the number of approvals required const finalizedApprovals = formattedRequests.filter( - (req) => req.privilegeId || req.reviewers.some((r) => r.status === ApprovalStatus.REJECTED) + (req) => + req.privilegeId || + req.reviewers.some((r) => r.status === ApprovalStatus.REJECTED) || + req.status !== ApprovalStatus.PENDING ); return { pendingCount: pendingApprovals.length, finalizedCount: finalizedApprovals.length }; 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 6d20369cf..1ceba3369 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 @@ -204,7 +204,7 @@ export const accessApprovalRequestServiceFactory = ({ const isRejected = reviewers.some((reviewer) => reviewer.status === ApprovalStatus.REJECTED); - if (!isRejected) { + if (!isRejected && duplicateRequest.status === ApprovalStatus.PENDING) { throw new BadRequestError({ message: "You already have a pending access request with the same criteria" }); } } @@ -478,7 +478,11 @@ export const accessApprovalRequestServiceFactory = ({ ); privilegeIdToSet = privilege.id; } - await accessApprovalRequestDAL.updateById(accessApprovalRequest.id, { privilegeId: privilegeIdToSet }, tx); + await accessApprovalRequestDAL.updateById( + accessApprovalRequest.id, + { privilegeId: privilegeIdToSet, status: ApprovalStatus.APPROVED }, + tx + ); } } diff --git a/backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts b/backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts index 965e25344..f4d7f4e6a 100644 --- a/backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts +++ b/backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts @@ -9,6 +9,7 @@ import { UnpackedPermissionSchema } from "@app/server/routes/sanitizedSchema/per import { ActorType } from "@app/services/auth/auth-type"; import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal"; +import { TAccessApprovalRequestDALFactory } from "../access-approval-request/access-approval-request-dal"; import { constructPermissionErrorMessage, validatePrivilegeChangeOperation } from "../permission/permission-fns"; import { TPermissionServiceFactory } from "../permission/permission-service"; import { @@ -16,6 +17,7 @@ import { ProjectPermissionSet, ProjectPermissionSub } from "../permission/project-permission"; +import { ApprovalStatus } from "../secret-approval-request/secret-approval-request-types"; import { TProjectUserAdditionalPrivilegeDALFactory } from "./project-user-additional-privilege-dal"; import { ProjectUserAdditionalPrivilegeTemporaryMode, @@ -30,6 +32,7 @@ type TProjectUserAdditionalPrivilegeServiceFactoryDep = { projectUserAdditionalPrivilegeDAL: TProjectUserAdditionalPrivilegeDALFactory; projectMembershipDAL: Pick; permissionService: Pick; + accessApprovalRequestDAL: Pick; }; export type TProjectUserAdditionalPrivilegeServiceFactory = ReturnType< @@ -44,7 +47,8 @@ const unpackPermissions = (permissions: unknown) => export const projectUserAdditionalPrivilegeServiceFactory = ({ projectUserAdditionalPrivilegeDAL, projectMembershipDAL, - permissionService + permissionService, + accessApprovalRequestDAL }: TProjectUserAdditionalPrivilegeServiceFactoryDep) => { const create = async ({ slug, @@ -279,6 +283,15 @@ export const projectUserAdditionalPrivilegeServiceFactory = ({ }); ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionMemberActions.Edit, ProjectPermissionSub.Member); + await accessApprovalRequestDAL.update( + { + privilegeId: userPrivilege.id + }, + { + privilegeDeletedAt: new Date(), + status: ApprovalStatus.REJECTED + } + ); const deletedPrivilege = await projectUserAdditionalPrivilegeDAL.deleteById(userPrivilege.id); return { ...deletedPrivilege, diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 0402c1a32..cb74cd44d 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -794,7 +794,8 @@ export const registerRoutes = async ( const projectUserAdditionalPrivilegeService = projectUserAdditionalPrivilegeServiceFactory({ permissionService, projectMembershipDAL, - projectUserAdditionalPrivilegeDAL + projectUserAdditionalPrivilegeDAL, + accessApprovalRequestDAL }); const projectKeyService = projectKeyServiceFactory({ permissionService,